1. 06 Sep, 2019 3 commits
  2. 05 Sep, 2019 3 commits
    • Tom Lane's avatar
      Use data directory inode number, not port, to select SysV resource keys. · 7de19fbc
      Tom Lane authored
      This approach provides a much tighter binding between a data directory
      and the associated SysV shared memory block (and SysV or named-POSIX
      semaphores, if we're using those).  Key collisions are still possible,
      but only between data directories stored on different filesystems,
      so the situation should be negligible in practice.  More importantly,
      restarting the postmaster with a different port number no longer
      risks failing to identify a relevant shared memory block, even when
      postmaster.pid has been removed.  A standalone backend is likewise
      much more certain to detect conflicting leftover backends.
      
      (In the longer term, we might now think about deprecating the port as
      a cluster-wide value, so that one postmaster could support sockets
      with varying port numbers.  But that's for another day.)
      
      The hazards fixed here apply only on Unix systems; our Windows code
      paths already use identifiers derived from the data directory path
      name rather than the port.
      
      src/test/recovery/t/017_shm.pl, which intends to test key-collision
      cases, has been substantially rewritten since it can no longer use
      two postmasters with identical port numbers to trigger the case.
      Instead, use Perl's IPC::SharedMem module to create a conflicting
      shmem segment directly.  The test script will be skipped if that
      module is not available.  (This means that some older buildfarm
      members won't run it, but I don't think that that results in any
      meaningful coverage loss.)
      
      Patch by me; thanks to Noah Misch and Peter Eisentraut for discussion
      and review.
      
      Discussion: https://postgr.es/m/16908.1557521200@sss.pgh.pa.us
      7de19fbc
    • Robert Haas's avatar
      Split tuptoaster.c into three separate files. · 8b94dab0
      Robert Haas authored
      detoast.c/h contain functions required to detoast a datum, partially
      or completely, plus a few other utility functions for examining the
      size of toasted datums.
      
      toast_internals.c/h contain functions that are used internally to the
      TOAST subsystem but which (mostly) do not need to be accessed from
      outside.
      
      heaptoast.c/h contains code that is intrinsically specific to the
      heap AM, either because it operates on HeapTuples or is based on the
      layout of a heap page.
      
      detoast.c and toast_internals.c are placed in
      src/backend/access/common rather than src/backend/access/heap.  At
      present, both files still have dependencies on the heap, but that will
      be improved in a future commit.
      
      Patch by me, reviewed and tested by Prabhat Sabu, Thomas Munro,
      Andres Freund, and Álvaro Herrera.
      
      Discussion: http://postgr.es/m/CA+TgmoZv-=2iWM4jcw5ZhJeL18HF96+W1yJeYrnGMYdkFFnEpQ@mail.gmail.com
      8b94dab0
    • 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
  3. 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
  4. 03 Sep, 2019 8 commits
  5. 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
  6. 01 Sep, 2019 1 commit
  7. 31 Aug, 2019 2 commits
  8. 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
  9. 29 Aug, 2019 2 commits
  10. 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
  11. 27 Aug, 2019 8 commits