1. 24 Dec, 2020 2 commits
    • Tom Lane's avatar
      Improve client error messages for immediate-stop situations. · 7e784d1d
      Tom Lane authored
      Up to now, if the DBA issued "pg_ctl stop -m immediate", the message
      sent to clients was the same as for a crash-and-restart situation.
      This is confusing, not least because the message claims that the
      database will soon be up again, something we have no business
      predicting.
      
      Improve things so that we can generate distinct messages for the two
      cases (and also recognize an ad-hoc SIGQUIT, should somebody try that).
      To do that, add a field to pmsignal.c's shared memory data structure
      that the postmaster sets just before broadcasting SIGQUIT to its
      children.  No interlocking seems to be necessary; the intervening
      signal-sending and signal-receipt should sufficiently serialize accesses
      to the field.  Hence, this isn't any riskier than the existing usages
      of pmsignal.c.
      
      We might in future extend this idea to improve other
      postmaster-to-children signal scenarios, although none of them
      currently seem to be as badly overloaded as SIGQUIT.
      
      Discussion: https://postgr.es/m/559291.1608587013@sss.pgh.pa.us
      7e784d1d
    • Michael Paquier's avatar
      Fix typos and grammar in docs and comments · 90fbf7c5
      Michael Paquier authored
      This fixes several areas of the documentation and some comments in
      matters of style, grammar, or even format.
      
      Author: Justin Pryzby
      Discussion: https://postgr.es/m/20201222041153.GK30237@telsasoft.com
      90fbf7c5
  2. 23 Dec, 2020 2 commits
    • Bruce Momjian's avatar
      dummy commit · 6ecf488d
      Bruce Momjian authored
      6ecf488d
    • Michael Paquier's avatar
      Fix portability issues with parsing of recovery_target_xid · 6db27037
      Michael Paquier authored
      The parsing of this parameter has been using strtoul(), which is not
      portable across platforms.  On most Unix platforms, unsigned long has a
      size of 64 bits, while on Windows it is 32 bits.  It is common in
      recovery scenarios to rely on the output of txid_current() or even the
      newer pg_current_xact_id() to get a transaction ID for setting up
      recovery_target_xid.  The value returned by those functions includes the
      epoch in the computed result, which would cause strtoul() to fail where
      unsigned long has a size of 32 bits once the epoch is incremented.
      
      WAL records and 2PC data include only information about 32-bit XIDs and
      it is not possible to have XIDs across more than one epoch, so
      discarding the high bits from the transaction ID set has no impact on
      recovery.  On the contrary, the use of strtoul() prevents a consistent
      behavior across platforms depending on the size of unsigned long.
      
      This commit changes the parsing of recovery_target_xid to use
      pg_strtouint64() instead, available down to 9.6.  There is one TAP test
      stressing recovery with recovery_target_xid, where a tweak based on
      pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets
      tested, as per an idea from Alexander Lakhin.
      
      Reported-by: Alexander Lakhin
      Discussion: https://postgr.es/m/16780-107fd0c0385b1035@postgresql.org
      Backpatch-through: 9.6
      6db27037
  3. 22 Dec, 2020 3 commits
    • Tom Lane's avatar
      Improve autoprewarm's handling of early-shutdown scenarios. · ff769831
      Tom Lane authored
      Bad things happen if the DBA issues "pg_ctl stop -m fast" before
      autoprewarm finishes loading its list of blocks to prewarm.
      The current worker process successfully terminates early, but
      (if this wasn't the last database with blocks to prewarm) the
      leader process will just try to launch another worker for the
      next database.  Since the postmaster is now in PM_WAIT_BACKENDS
      state, it ignores the launch request, and the leader just sits
      until it's killed manually.
      
      This is mostly the fault of our half-baked design for launching
      background workers, but a proper fix for that is likely to be
      too invasive to be back-patchable.  To ameliorate the situation,
      fix apw_load_buffers() to check whether SIGTERM has arrived
      just before trying to launch another worker.  That leaves us with
      only a very narrow window in each worker launch where SIGTERM
      could occur between the launch request and successful worker start.
      
      Another issue is that if the leader process does manage to exit,
      it unconditionally rewrites autoprewarm.blocks with only the
      blocks currently in shared buffers, thus forgetting any blocks
      that we hadn't reached yet while prewarming.  This seems quite
      unhelpful, since the next database start will then not have the
      expected prewarming benefit.  Fix it to not modify the file if
      we shut down before the initial load attempt is complete.
      
      Per bug #16785 from John Thompson.  Back-patch to v11 where
      the autoprewarm code was introduced.
      
      Discussion: https://postgr.es/m/16785-c0207d8c67fb5f25@postgresql.org
      ff769831
    • Tom Lane's avatar
      Increase timeout in 021_row_visibility.pl. · 08dde1b3
      Tom Lane authored
      Commit 7b28913b figured 30 seconds is long enough for anybody,
      but in contexts like valgrind runs, it isn't necessarily.
      08dde1b3
    • Tomas Vondra's avatar
      Improve find_em_expr_usable_for_sorting_rel comment · 1ca2eb10
      Tomas Vondra authored
      Clarify the relationship between find_em_expr_usable_for_sorting_rel and
      prepare_sort_from_pathkeys, i.e. what restrictions need to be shared
      between those two places.
      
      Author: James Coleman
      Reviewed-by: Tomas Vondra
      Backpatch-through: 13
      Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
      1ca2eb10
  4. 21 Dec, 2020 7 commits
  5. 20 Dec, 2020 7 commits
  6. 19 Dec, 2020 2 commits
  7. 18 Dec, 2020 3 commits
    • Tom Lane's avatar
      Add a couple of missed .gitignore entries. · 8afca702
      Tom Lane authored
      Any subdirectory that's ignoring /output_iso/ should also
      ignore /tmp_check_iso/, which could be left behind by a
      failed pg_isolation_regress_check run.
      
      I think these have been wrong for awhile, but it doesn't
      seem important to fix in back branches.
      8afca702
    • Tom Lane's avatar
      Avoid memcpy() with same source and destination during relmapper init. · 53d4f5fe
      Tom Lane authored
      A narrow reading of the C standard says that memcpy(x,x,n) is undefined,
      although it's hard to envision an implementation that would really
      misbehave.  However, analysis tools such as valgrind might whine about
      this; accordingly, let's band-aid relmapper.c to not do it.
      
      See also 5b630501, d3f4e8a8, ad7b48ea, and other similar fixes.
      Apparently, none of those folk tried valgrinding initdb?  This has been
      like this for long enough that I'm surprised it hasn't been reported
      before.
      
      Back-patch, just in case anybody wants to use a back branch on a platform
      that complains about this; we back-patched those earlier fixes too.
      
      Discussion: https://postgr.es/m/161790.1608310142@sss.pgh.pa.us
      53d4f5fe
    • Fujii Masao's avatar
      pg_stat_statements: Track time at which all statistics were last reset. · 2e0fedf0
      Fujii Masao authored
      This commit adds "stats_reset" column into the pg_stat_statements_info
      view. This column indicates the time at which all statistics in the
      pg_stat_statements view were last reset.
      
      Per discussion, this commit also changes pg_stat_statements_info code
      so that "dealloc" column is reset at the same time as "stats_reset" is reset,
      i.e., whenever all pg_stat_statements entries are removed, for the sake
      of consistency. Previously "dealloc" was reset only when
      pg_stat_statements_reset(0, 0, 0) is called and was not reset when
      pg_stat_statements_reset() with non-zero value argument discards all
      entries. This was confusing.
      
      Author: Naoki Nakamichi, Yuki Seino
      Reviewed-by: Yuki Seino, Kyotaro Horiguchi, Li Japin, Fujii Masao
      Discussion: https://postgr.es/m/c102cf3180d0ee73c1c5a0f7f8558322@oss.nttdata.com
      2e0fedf0
  8. 17 Dec, 2020 1 commit
  9. 16 Dec, 2020 3 commits
  10. 15 Dec, 2020 6 commits
    • Peter Geoghegan's avatar
      Remove obsolete btrescan() comment. · 41ddc27f
      Peter Geoghegan authored
      "Ordering stuff" refered to a _bt_first() call to _bt_orderkeys().
      However, the _bt_orderkeys() function was renamed to
      _bt_preprocess_keys() by commit fa5c8a05.
      
      _bt_preprocess_keys() is directly referenced just after the removed
      comment already, which seems sufficient.
      41ddc27f
    • Alvaro Herrera's avatar
      Remove useless variable stores · a18422a3
      Alvaro Herrera authored
      Mistakenly introduced in 4cbe3ac3; bug repaired in 148e632c but
      the stores were accidentally.
      a18422a3
    • Tomas Vondra's avatar
      Error out when Gather Merge input is not sorted · 6bc27698
      Tomas Vondra authored
      To build Gather Merge path, the input needs to be sufficiently sorted.
      Ensuring this is the responsibility of the code constructing the paths,
      but create_gather_merge_plan tried to handle unsorted paths by adding
      an explicit Sort. In light of the recent issues related to Incremental
      Sort, this is rather fragile. Some of the expressions may be volatile
      or parallel unsafe, in which case we can't add the Sort here.
      
      We could do more checks and add the Sort in at least some cases, but
      it seems cleaner to just error out and make it clear this is a bug in
      code constructing those paths.
      
      Author: James Coleman
      Reviewed-by: Tomas Vondra
      Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
      Discussion: https://postgr.es/m/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com
      6bc27698
    • Peter Eisentraut's avatar
      Clean up ancient test style · c06d6aa4
      Peter Eisentraut authored
      Many older tests where written in a style like
      
          SELECT '' AS two, i.* FROM INT2_TBL
      
      where the first column indicated the number of expected result rows.
      This has gotten increasingly out of date, as the test data fixtures
      have expanded, so a lot of these were wrong and misleading.  Moreover,
      this style isn't really necessary, since the psql output already shows
      the number of result rows.
      
      To clean this up, remove all those extra columns.
      
      Discussion: https://www.postgresql.org/message-id/flat/1a25312b-2686-380d-3c67-7a69094a999f%40enterprisedb.com
      c06d6aa4
    • Tom Lane's avatar
      Improve hash_create()'s API for some added robustness. · b3817f5f
      Tom Lane authored
      Invent a new flag bit HASH_STRINGS to specify C-string hashing, which
      was formerly the default; and add assertions insisting that exactly
      one of the bits HASH_STRINGS, HASH_BLOBS, and HASH_FUNCTION be set.
      This is in hopes of preventing recurrences of the type of oversight
      fixed in commit a1b8aa1e (i.e., mistakenly omitting HASH_BLOBS).
      
      Also, when HASH_STRINGS is specified, insist that the keysize be
      more than 8 bytes.  This is a heuristic, but it should catch
      accidental use of HASH_STRINGS for integer or pointer keys.
      (Nearly all existing use-cases set the keysize to NAMEDATALEN or
      more, so there's little reason to think this restriction should
      be problematic.)
      
      Tweak hash_create() to insist that the HASH_ELEM flag be set, and
      remove the defaults it had for keysize and entrysize.  Since those
      defaults were undocumented and basically useless, no callers
      omitted HASH_ELEM anyway.
      
      Also, remove memset's zeroing the HASHCTL parameter struct from
      those callers that had one.  This has never been really necessary,
      and while it wasn't a bad coding convention it was confusing that
      some callers did it and some did not.  We might as well save a few
      cycles by standardizing on "not".
      
      Also improve the documentation for hash_create().
      
      In passing, improve reinit.c's usage of a hash table by storing
      the key as a binary Oid rather than a string; and, since that's
      a temporary hash table, allocate it in CurrentMemoryContext for
      neatness.
      
      Discussion: https://postgr.es/m/590625.1607878171@sss.pgh.pa.us
      b3817f5f
    • Jeff Davis's avatar
      Revert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE." · a58db3aa
      Jeff Davis authored
      This reverts commit 3a9e64aa.
      
      Commit 4bad60e3 fixed the root of the problem that 3a9e64aa worked
      around.
      
      This enables proper pipelining of commands after terminating
      replication, eliminating an undocumented limitation.
      
      Discussion: https://postgr.es/m/3d57bc29-4459-578b-79cb-7641baf53c57%40iki.fi
      Backpatch-through: 9.5
      a58db3aa
  11. 14 Dec, 2020 2 commits
    • Michael Paquier's avatar
      Improve some code around cryptohash functions · 9b584953
      Michael Paquier authored
      This adjusts some code related to recent changes for cryptohash
      functions:
      - Add a variable in md5.h to track down the size of a computed result,
      moved from pgcrypto.  Note that pg_md5_hash() assumed a result of this
      size already.
      - Call explicit_bzero() on the hashed data when freeing the context for
      fallback implementations.  For MD5, particularly, it would be annoying
      to leave some non-zeroed data around.
      - Clean up some code related to recent changes of uuid-ossp.  .gitignore
      still included md5.c and a comment was incorrect.
      
      Discussion: https://postgr.es/m/X9HXKTgrvJvYO7Oh@paquier.xyz
      9b584953
    • Michael Paquier's avatar
      Add some checkpoint/restartpoint status to ps display · df9274ad
      Michael Paquier authored
      This is done for end-of-recovery and shutdown checkpoints/restartpoints
      (end-of-recovery restartpoints don't exist) rather than all types of
      checkpoints, in cases where it may not be possible to rely on
      pg_stat_activity to get a status from the startup or checkpointer
      processes.
      
      For example, at the end of a crash recovery, this is useful to know if a
      checkpoint is running in the startup process, while previously the ps
      display may only show some information about "recovering" something,
      that can be confusing while a checkpoint runs.
      
      Author: Justin Pryzby
      Reviewed-by: Nathan Bossart, Kirk Jamison, Fujii Masao, Michael Paquier
      Discussion: https://postgr.es/m/20200818225238.GP17022@telsasoft.com
      df9274ad
  12. 13 Dec, 2020 2 commits