Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 25 Oct, 2019 8 commits
    • Peter Eisentraut's avatar
      doc: Use proper em and en dashes · cbe63d02
      Peter Eisentraut authored
      cbe63d02
    • Tom Lane's avatar
      Revert "Revert part of commit dddf4cdc." · ee201520
      Tom Lane authored 
      This reverts commit c114229c.
Commit 1408d5d8 should make it
safe to include these headers in the natural order.
      ee201520
    • Tom Lane's avatar
      Get rid of useless/dangerous redefinition of bool in ECPG. · 1408d5d8
      Tom Lane authored 
      pgtypeslib_extern.h contained fallback definitions of "bool", "FALSE",
and "TRUE".  The latter two are just plain unused, and have been for
awhile.  The former came into play only if there wasn't a macro
definition of "bool", which is true only if we aren't using <stdbool.h>.
However, it then defined bool as "char"; since commit d26a810e that
conflicts with c.h's desire to use "unsigned char".  We'd missed seeing
any bad effects of that due to accidental header inclusion order choices,
but dddf4cdc exposed that it was problematic.

To fix, let's just get rid of these definitions.  They should not be
needed because everyplace in Postgres should be relying on c.h to
provide a definition for type bool.  (Note that despite its name,
pgtypeslib_extern.h isn't exposed to any outside code; we don't
install it.)

This doesn't fully resolve the issue, because ecpglib.h is doing
similar things, but that seems to require more thought to fix.

Back-patch to v12 where d26a810e came in, to forestall any unpleasant
surprises from future back-patched bug fixes.

Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com
      1408d5d8
    • Tom Lane's avatar
      Improve management of statement timeouts. · 22f6f2c1
      Tom Lane authored 
      Commit f8e5f156 added private state in postgres.c to track whether
a statement timeout is running.  This seems like bad design to me;
timeout.c's private state should be the single source of truth about
that.  We already fixed one bug associated with failure to keep those
states in sync (cf. be42015f), and I've got little faith that we
won't find more in future.  So get rid of postgres.c's local variable
by exposing a way to ask timeout.c whether a timeout is running.
(Obviously, such an inquiry is subject to race conditions, but it
seems fine for the purpose at hand.)

To make get_timeout_active() as cheap as possible, add a flag in
the per-timeout struct showing whether that timeout is active.
This allows some small savings elsewhere in timeout.c, mainly
elimination of unnecessary searches of the active_timeouts array.

While at it, fix enable_statement_timeout to not call disable_timeout
when statement_timeout is 0 and the timeout is not running.  This
avoids a useless deschedule-and-reschedule-timeouts cycle, which
represents a significant savings (at least one kernel call) when
there is any other active timeout.  Right now, there usually isn't,
but there are proposals around to change that.

Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org
      22f6f2c1
    • Tom Lane's avatar
      Reset statement_timeout between queries of a multi-query string. · 2b2bacdc
      Tom Lane authored 
      Historically, we started the timer (if StatementTimeout > 0) at the
beginning of a simple-Query message and usually let it run until the
end, so that the timeout limit applied to the entire query string,
and intra-string changes of the statement_timeout GUC had no effect.
But, confusingly, a COMMIT within the string would reset the state
and allow a fresh timeout cycle to start with the current setting.

Commit f8e5f156 changed the behavior of statement_timeout for extended
query protocol, and as an apparently-unintended side effect, a change in
the statement_timeout GUC during a multi-statement simple-Query message
might have an effect immediately --- but only if it was going from
"disabled" to "enabled".

This is all pretty confusing, not to mention completely undocumented.
Let's change things so that the timeout is always reset between queries
of a multi-query string, whether they're transaction control commands
or not.  Thus the active timeout setting is applied to each query in
the string, separately.  This costs a few more cycles if statement_timeout
is active, but it provides much more intuitive behavior, especially if one
changes statement_timeout in one of the queries of the string.

Also, add something to the documentation to explain all this.

Per bug #16035 from Raj Mohite.  Although this is a bug fix, I'm hesitant
to back-patch it; conceivably somebody has worked out the old behavior
and is depending on it.  (But note that this change should make the
behavior less restrictive in most cases, since the timeout will now
be applied to shorter segments of code.)

Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org
      2b2bacdc
    • Amit Kapila's avatar
      Revert part of commit dddf4cdc. · c114229c
      Amit Kapila authored 
      The commit dddf4cdc tries to ensure that the Postgres header file
inclusions are in order based on their ASCII value.  However, in one of
the case there is a header file dependency due to which we can't maintain
such order.

Author: Amit Kapila
Discussion: https://postgr.es/m/E1iNpHW-000855-1u@gemulon.postgresql.org
      c114229c
    • Amit Kapila's avatar
      Make the order of the header file includes consistent in non-backend modules. · dddf4cdc
      Amit Kapila authored 
      Similar to commit 7e735035, this commit makes the order of header file
inclusion consistent for non-backend modules.

In passing, fix the case where we were using angle brackets (<>) for the
local module includes instead of quotes ("").

Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
      dddf4cdc
    • Michael Paquier's avatar
      Handle interrupts within a transaction context in REINDEX CONCURRENTLY · 8270a0d9
      Michael Paquier authored 
      Phases 2 (building the new index) and 3 (validating the new index)
checked for interrupts outside a transaction context, having as
consequence to not release session-level locks taken on the parent
relation and the old and new indexes processed.  This could for example
be triggered with statement_timeout and a bad timing, and would issue
confusing error messages when shutting down the session still holding
the locks (note that an assertion failure would be triggered first), on
top of more issues with concurrent sessions trying to take a lock that
would interfere with the SHARE UPDATE EXCLUSIVE locks hold here.

This moves all the interruption checks inside a transaction context.
Note that I have manually tested all interruptions to make sure that
invalid indexes can be cleaned up properly.  Partition indexes still
have issues on their own with some missing dependency handling, which
will be dealt with in a follow-up patch.

Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/20191013025145.GC4475@telsasoft.com
Backpatch-through: 12
      8270a0d9
  3. 24 Oct, 2019 3 commits
  5. 23 Oct, 2019 6 commits
  7. 22 Oct, 2019 2 commits
  9. 21 Oct, 2019 9 commits
  11. 20 Oct, 2019 1 commit
  13. 19 Oct, 2019 5 commits
  15. 18 Oct, 2019 5 commits
  17. 17 Oct, 2019 1 commit
    • Alvaro Herrera's avatar
      Fix minor bug in logical-replication walsender shutdown · 38ddeab1
      Alvaro Herrera authored 
      Logical walsender should exit when it catches up with sending WAL during
shutdown; but there was a rare corner case when it failed to because of
a race condition that puts it back to wait for more WAL instead -- but
since there wasn't any, it'd not shut down immediately.  It would only
continue the shutdown when wal_sender_timeout terminates the sleep,
which causes annoying waits during shutdown procedure.  Restructure the
code so that we no longer forget to set WalSndCaughtUp in that case.

This was an oversight in commit c6c33343.

Backpatch all the way down to 9.4.

Author: Craig Ringer, Álvaro Herrera
Discussion: https://postgr.es/m/CAMsr+YEuz4XwZX_QmnX_-2530XhyAmnK=zCmicEnq1vLr0aZ-g@mail.gmail.com
      38ddeab1