1. 05 Sep, 2019 1 commit
    • Peter Eisentraut's avatar
      Use explicit_bzero · 74a308cf
      Peter Eisentraut authored
      Use the explicit_bzero() function in places where it is important that
      security information such as passwords is cleared from memory.  There
      might be other places where it could be useful; this is just an
      initial collection.
      
      For platforms that don't have explicit_bzero(), provide various
      fallback implementations.  (explicit_bzero() itself isn't standard,
      but as Linux/glibc, FreeBSD, and OpenBSD have it, it's the most common
      spelling, so it makes sense to make that the invocation point.)
      
      Discussion: https://www.postgresql.org/message-id/flat/42d26bde-5d5b-c90d-87ae-6cab875f73be%402ndquadrant.com
      74a308cf
  2. 04 Sep, 2019 2 commits
    • Michael Paquier's avatar
      Fix thinko when ending progress report for a backend · ae060a52
      Michael Paquier authored
      The logic ending progress reporting for a backend entry introduced by
      b6fb6471 causes callers of pgstat_progress_end_command() to do some extra
      work when track_activities is enabled as the process fields are reset in
      the backend entry even if no command were started for reporting.
      
      This resets the fields only if a command is registered for progress
      reporting, and only if track_activities is enabled.
      
      Author: Masahiho Sawada
      Discussion: https://postgr.es/m/CAD21AoCry_vJ0E-m5oxJXGL3pnos-xYGCzF95rK5Bbi3Uf-rpA@mail.gmail.com
      Backpatch-through: 9.6
      ae060a52
    • Michael Paquier's avatar
      Delay fsyncs of pg_basebackup until the end of backup · 522baf14
      Michael Paquier authored
      Since the addition of fsync requests in bc34223b to make base backup data
      consistent on disk once pg_basebackup finishes, each tablespace tar file
      is individually flushed once completed, with an additional flush of the
      parent directory when the base backup finishes.  While holding a
      connection to the server, a fsync request taking a long time may cause a
      failure of the base backup, which is annoying for any integration.  A
      recent example of breakage can involve tcp_user_timeout, but
      wal_sender_timeout can cause similar problems.
      
      While reviewing the code, there was a second issue causing too many
      fsync requests to be done for the same WAL data.  As recursive fsyncs
      are done at the end of the backup for both the plain and tar formats
      from the base target directory where everything is written, it is fine
      to disable fsyncs when fetching or streaming WAL.
      
      Reported-by: Ryohei Takahashi
      Author: Michael Paquier
      Reviewed-by: Ryohei Takahashi
      Discussion: https://postgr.es/m/OSBPR01MB4550DAE2F8C9502894A45AAB82BE0@OSBPR01MB4550.jpnprd01.prod.outlook.com
      Backpatch-through: 10
      522baf14
  3. 03 Sep, 2019 8 commits
  4. 02 Sep, 2019 4 commits
    • Tom Lane's avatar
      Avoid touching replica identity index in ExtractReplicaIdentity(). · f63a5ead
      Tom Lane authored
      In what seems like a fit of misplaced optimization,
      ExtractReplicaIdentity() accessed the relation's replica-identity
      index without taking any lock on it.  Usually, the surrounding query
      already holds some lock so this is safe enough ... but in the case
      of a previously-planned delete, there might be no existing lock.
      Given a suitable test case, this is exposed in v12 and HEAD by an
      assertion added by commit b04aeb0a.
      
      The whole thing's rather poorly thought out anyway; rather than
      looking directly at the index, we should use the index-attributes
      bitmap that's held by the parent table's relcache entry, as the
      caller functions do.  This is more consistent and likely a bit
      faster, since it avoids a cache lookup.  Hence, change to doing it
      that way.
      
      While at it, rather than blithely assuming that the identity
      columns are non-null (with catastrophic results if that's wrong),
      add assertion checks that they aren't null.  Possibly those should
      be actual test-and-elog, but I'll leave it like this for now.
      
      In principle, this is a bug that's been there since this code was
      introduced (in 9.4).  In practice, the risk seems quite low, since
      we do have a lock on the index's parent table, so concurrent
      changes to the index's catalog entries seem unlikely.  Given the
      precedent that commit 9c703c16 wasn't back-patched, I won't risk
      back-patching this further than v12.
      
      Per report from Hadi Moshayedi.
      
      Discussion: https://postgr.es/m/CAK=1=Wrek44Ese1V7LjKiQS-Nd-5LgLi_5_CskGbpggKEf3tKQ@mail.gmail.com
      f63a5ead
    • Tom Lane's avatar
      Handle corner cases correctly in psql's reconnection logic. · aef36238
      Tom Lane authored
      After an unexpected connection loss and successful reconnection,
      psql neglected to resynchronize its internal state about the server,
      such as server version.  Ordinarily we'd be reconnecting to the same
      server and so this isn't really necessary, but there are scenarios
      where we do need to update --- one example is where we have a list
      of possible connection targets and they're not all alike.
      
      Define "resynchronize" as including connection_warnings(), so that
      this case acts the same as \connect.  This seems useful; for example,
      if the server version did change, the user might wish to know that.
      An attuned user might also notice that the new connection isn't
      SSL-encrypted, for example, though this approach isn't especially
      in-your-face about such changes.  Although this part is a behavioral
      change, it only affects interactive sessions, so it should not break
      any applications.
      
      Also, in do_connect, make sure that we desynchronize correctly when
      abandoning an old connection in non-interactive mode.
      
      These problems evidently are the result of people patching only one
      of the two places where psql deals with connection changes, so insert
      some cross-referencing comments in hopes of forestalling future bugs
      of the same ilk.
      
      Lastly, in Windows builds, issue codepage mismatch warnings only at
      startup, not during reconnections.  psql's codepage can't change
      during a reconnect, so complaining about it again seems like useless
      noise.
      
      Peter Billen and Tom Lane.  Back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/CAMTXbE8e6U=EBQfNSe01Ej17CBStGiudMAGSOPaw-ALxM-5jXg@mail.gmail.com
      aef36238
    • Alvaro Herrera's avatar
      Add POD documentation to TestLib.pm · 6fcc40b1
      Alvaro Herrera authored
      This module was pretty much undocumented.  Fix that.
      
      Inspired by a preliminary patch sent by Ramanarayana, heavily updated by
      Andrew Dunstan, and reviewed by Michael Paquier.
      
      Discussion: https://postgr.es/m/CAF6A77G_WJTwBV9SBxCnQfZB09hm1p1O3stZ6eE5QiYd=X84Jg@mail.gmail.com
      6fcc40b1
    • Michael Paquier's avatar
      Add overflow-safe math inline functions for unsigned integers · 7dedfd22
      Michael Paquier authored
      Similarly to the signed versions added in 4d6ad312, this adds a set of
      inline functions for overflow checks with unsigned integers, including
      uint16, uint32 and uint64.  This relies on compiler built-in overflow
      checks by default if available.  The behavior of unsigned integers is
      well-defined so the fallback implementations checks are simple for
      additions and subtractions.  Multiplications avoid division-based checks
      which are expensive if possible, still this can happen for uint64 if
      128-bit integers are not available.
      
      While on it, the code in common/int.h is reorganized to avoid too many
      duplicated comments.  The new macros will be used in a follow-up patch.
      
      All thanks to Andres Freund for the input provided.
      
      Author: Fabien Coelho, Michael Paquier
      Discussion: https://postgr.es/m/20190830073423.GB2354@paquier.xyz
      7dedfd22
  5. 01 Sep, 2019 1 commit
  6. 31 Aug, 2019 2 commits
  7. 30 Aug, 2019 2 commits
    • Tom Lane's avatar
      Doc: restructure documentation of the configure script's options. · 137b03b8
      Tom Lane authored
      The list of configure options has grown long, and there was next
      to no organization to it, never mind any indication of which options
      were interesting to most people.  Break it into several sub-sections
      to provide a bit of structure, and add some introductory text where
      it seems helpful to point people to particular options.
      
      I failed to resist the temptation to do a small amount of
      word-smithing on some of the option descriptions, too.
      But mostly this is reorganization and addition of intro text.
      
      Discussion: https://postgr.es/m/6384.1559917369@sss.pgh.pa.us
      137b03b8
    • Tom Lane's avatar
      Doc: remove some long-obsolete information from installation.sgml. · 76c2af92
      Tom Lane authored
      Section 16.2 pointed to platform-specific FAQ files that we removed
      way back in 8.4.  Section 16.7 contained a bunch of information about
      AIX and HPUX bugs that were squashed decades ago, plus discussions of
      old compiler versions that are certainly moot now that we require C99
      support.  Since we're obviously not maintaining this stuff carefully,
      just remove it.  The HPUX sub-section seems like it can go away
      entirely, since everything it said that was still applicable was
      redundant with material elsewhere in the chapter.
      
      In passing, I couldn't resist the temptation to do a small amount
      of copy-editing on nearby text.
      
      Back-patch to v12, since this stuff is surely obsolete in any
      branch that requires C99.
      
      Discussion: https://postgr.es/m/15538.1567042743@sss.pgh.pa.us
      76c2af92
  8. 29 Aug, 2019 2 commits
  9. 28 Aug, 2019 5 commits
    • Tom Lane's avatar
      744c848d
    • Heikki Linnakangas's avatar
      Fix overflow check and comment in GIN posting list encoding. · bde7493d
      Heikki Linnakangas authored
      The comment did not match what the code actually did for integers with
      the 43rd bit set. You get an integer like that, if you have a posting
      list with two adjacent TIDs that are more than 2^31 blocks apart.
      According to the comment, we would store that in 6 bytes, with no
      continuation bit on the 6th byte, but in reality, the code encodes it
      using 7 bytes, with a continuation bit on the 6th byte as normal.
      
      The decoding routine also handled these 7-byte integers correctly, except
      for an overflow check that assumed that one integer needs at most 6 bytes.
      Fix the overflow check, and fix the comment to match what the code
      actually does. Also fix the comment that claimed that there are 17 unused
      bits in the 64-bit representation of an item pointer. In reality, there
      are 64-32-11=21.
      
      Fitting any item pointer into max 6 bytes was an important property when
      this was written, because in the old pre-9.4 format, item pointers were
      stored as plain arrays, with 6 bytes for every item pointer. The maximum
      of 6 bytes per integer in the new format guaranteed that we could convert
      any page from the old format to the new format after upgrade, so that the
      new format was never larger than the old format. But we hardly need to
      worry about that anymore, and running into that problem during upgrade,
      where an item pointer is expanded from 6 to 7 bytes such that the data
      doesn't fit on a page anymore, is implausible in practice anyway.
      
      Backpatch to all supported versions.
      
      This also includes a little test module to test these large distances
      between item pointers, without requiring a 16 TB table. It is not
      backpatched, I'm including it more for the benefit of future development
      of new posting list formats.
      
      Discussion: https://www.postgresql.org/message-id/33bfc20a-5c86-f50c-f5a5-58e9925d05ff%40iki.fi
      Reviewed-by: Masahiko Sawada, Alexander Korotkov
      bde7493d
    • Thomas Munro's avatar
      Avoid catalog lookups in RelationAllowsEarlyPruning(). · 720b59b5
      Thomas Munro authored
      RelationAllowsEarlyPruning() performed a catalog scan, but is used
      in two contexts where that was a bad idea:
      
      1.  In heap_page_prune_opt(), which runs very frequently in some large
          scans.  This caused major performance problems in a field report
          that was easy to reproduce.
      
      2.  In TestForOldSnapshot(), which runs while we hold a buffer content
          lock.  It's not clear if this was guaranteed to be free of buffer
          deadlock risk.
      
      The check was introduced in commit 2cc41acd and defended against a
      real problem: 9.6's hash indexes have no page LSN and so we can't
      allow early pruning (ie the snapshot-too-old feature).  We can remove
      the check from all later releases though: hash indexes are now logged,
      and there is no way to create UNLOGGED indexes on regular logged
      tables.
      
      If a future release allows such a combination, it might need to put
      a similar check in place, but it'll need some more thought.
      
      Back-patch to 10.
      
      Author: Thomas Munro
      Reviewed-by: Tom Lane, who spotted the second problem
      Discussion: https://postgr.es/m/CA%2BhUKGKT8oTkp5jw_U4p0S-7UG9zsvtw_M47Y285bER6a2gD%2Bg%40mail.gmail.com
      Discussion: https://postgr.es/m/CAA4eK1%2BWy%2BN4eE5zPm765h68LrkWc3Biu_8rzzi%2BOYX4j%2BiHRw%40mail.gmail.com
      720b59b5
    • Michael Paquier's avatar
      Improve coverage of utils/float.h · 80d0e5ba
      Michael Paquier authored
      check_float4_val() checks after underflow and overflow of values
      converted from float8 to float4, but there has never been any regression
      tests for that.  This brings the coverage of float.h to 100%.
      
      Author: Movead Li
      Discussion: https://postgr.es/m/20190822174636998766188@highgo.ca
      80d0e5ba
    • Michael Paquier's avatar
      Disable timeouts when running pg_rewind with online source cluster · be182e4f
      Michael Paquier authored
      In this case, the transfer uses a libpq connection, which is subject to
      the timeout parameters set at system level, and this can make the rewind
      operation suddenly canceled which is not good for automation.  One
      workaround to such issues would be to use PGOPTIONS to enforce the
      wanted timeout parameters, but that's annoying, and for example pg_dump,
      which can run potentially long-running queries disables all types of
      timeouts.
      
      lock_timeout and statement_timeout are the ones which can cause problems
      now.  Note that pg_rewind does not use transactions, so disabling
      idle_in_transaction_session_timeout is optional, but it feels safer to
      do so for the future.
      
      This is back-patched down to 9.5.  idle_in_transaction_session_timeout
      is only present since 9.6.
      
      Author: Alexander Kukushkin
      Discussion: https://postgr.es/m/CAFh8B=krcVXksxiwVQh1SoY+ziJ-JC=6FcuoBL3yce_40Es5_g@mail.gmail.com
      Backpatch-through: 9.5
      be182e4f
  10. 27 Aug, 2019 8 commits
  11. 26 Aug, 2019 5 commits
    • Tom Lane's avatar
      Fix 007_sync_rep.pl to notice failures in ALTER SYSTEM SET. · fb57f40e
      Tom Lane authored
      If a test case tried to set an invalid value of synchronous_standby_names,
      the test script didn't detect that, which seems like a bad idea.
      Noticed while testing a proposed patch that broke some of these
      test cases.
      fb57f40e
    • Tom Lane's avatar
      Fix postmaster state machine to handle dead_end child crashes better. · ee327823
      Tom Lane authored
      A report from Alvaro Herrera shows that if we're in PM_STARTUP
      state, and we spawn a dead_end child to reject some incoming
      connection request, and that child dies with an unexpected exit
      code, the postmaster does not respond well.  We correctly send
      SIGQUIT to the startup process, but then:
      
      * if the startup process exits with nonzero exit code, as expected,
      we thought that that indicated a crash and aborted startup.
      
      * if the startup process exits with zero exit code, which is possible
      due to the inherent race condition, we'd advance to PM_RUN state
      which is fine --- but the code forgot that AbortStartTime would be
      nonzero in this situation.  We'd either die on the Asserts saying
      that it was zero, or perhaps misbehave later on.  (A quick look
      suggests that the only misbehavior might be busy-waiting due to
      DetermineSleepTime doing the wrong thing.)
      
      To fix the first point, adjust the state-machine logic to recognize
      that a nonzero exit code is expected after sending SIGQUIT, and have
      it transition to a state where we can restart the startup process.
      To fix the second point, change the Asserts to clear the variable
      rather than just claiming it should be clear already.
      
      Perhaps we could improve this further by not treating a crash of
      a dead_end child as a reason for panic'ing the database.  However,
      since those child processes are connected to shared memory, that
      seems a bit risky.  There are few good reasons for a dead_end child
      to report failure anyway (the cause of this in Alvaro's report is
      quite unclear).  On balance, therefore, a minimal fix seems best.
      
      This is an oversight in commit 45811be9.  While that was back-patched,
      I'm hesitant to back-patch this change.  The lack of reasons for a
      dead_end child to fail suggests that the case should be very rare in
      the field, which squares with the lack of reports; so it seems like
      this might not be worth the risk of introducing new issues.  In any
      case we can let it bake awhile in HEAD before considering a back-patch.
      
      Discussion: https://postgr.es/m/20190615160950.GA31378@alvherre.pgsql
      ee327823
    • Tom Lane's avatar
      Make comment in fmgr.h match the one in fmgr.c. · 348778dd
      Tom Lane authored
      Incompletely quoting an API spec does nobody any good.  Noted by
      Paul Jungwirth.  Looks like the discrepancy was my fault originally :-(
      
      Discussion: https://postgr.es/m/CA+renyU_J8TU_d3Kr0PkuOgFbpypextendu7a+_d5NOfVdvDeA@mail.gmail.com
      348778dd
    • Peter Eisentraut's avatar
      Fix gettext triggers specification · f2690338
      Peter Eisentraut authored
      In cc8d4151, the arguments of
      warn_or_exit_horribly() were changed but this was not updated.
      f2690338
    • Andrew Dunstan's avatar
      Adjust to latest Msys2 kernel release number · c62b8443
      Andrew Dunstan authored
      Previously 'uname -r' on Msys2 reported a kernele release starting with
      2. The latest version starts with 3. In commit 1638623f we specifically
      looked for one starting with 2. This is now changed to look for any
      digit between 2 and 9.
      
      backpatch to release 10.
      c62b8443