1. 05 Jul, 2017 5 commits
  2. 04 Jul, 2017 2 commits
  3. 03 Jul, 2017 4 commits
  4. 02 Jul, 2017 5 commits
    • Tom Lane's avatar
      Fix bug in PostgresNode::query_hash's split() call. · efdb4f29
      Tom Lane authored
      By default, Perl's split() function drops trailing empty fields,
      which is not what we want here.  Oversight in commit fb093e4c.
      We'd managed to miss it thus far thanks to the very limited usage
      of this function.
      
      Discussion: https://postgr.es/m/14837.1499029831@sss.pgh.pa.us
      efdb4f29
    • Tom Lane's avatar
      Try to improve readability of recovery/t/009_twophase.pl test. · 4e15387d
      Tom Lane authored
      The original coding here was very confusing, because it named the
      two servers it set up "master" and "slave" even though it swapped
      their replication roles multiple times.  At any given point in the
      script it was very unobvious whether "$node_master" actually referred
      to the server named "master" or the other one.  Instead, pick arbitrary
      names for the two servers --- I used "london" and "paris" --- and
      distinguish those permanent names from the nonce references $cur_master
      and $cur_slave.  Add logging to help distinguish which is which at
      any given point.  Also, use distinct data and transaction names to
      make all the prepared transactions easily distinguishable in the
      postmaster logs.  (There was one place where we intentionally tested
      that the server could cope with re-use of a transaction name, but
      it seems like one place is sufficient for that purpose.)
      
      Also, add checks at the end to make sure that all the transactions
      that were supposed to be committed did survive.
      
      Discussion: https://postgr.es/m/28238.1499010855@sss.pgh.pa.us
      4e15387d
    • Tom Lane's avatar
      Improve TAP test function PostgresNode::poll_query_until(). · de3de0af
      Tom Lane authored
      Add an optional "expected" argument to override the default assumption
      that we're waiting for the query to return "t".  This allows replacing
      a handwritten polling loop in recovery/t/007_sync_rep.pl with use of
      poll_query_until(); AFAICS that's the only remaining ad-hoc polling
      loop in our TAP tests.
      
      Change poll_query_until() to probe ten times per second not once per
      second.  Like some similar changes I've been making recently, the
      one-second interval seems to be rooted in ancient traditions rather
      than the actual likely wait duration on modern machines.  I'd consider
      reducing it further if there were a convenient way to spawn just one
      psql for the whole loop rather than one per probe attempt.
      
      Discussion: https://postgr.es/m/12486.1498938782@sss.pgh.pa.us
      de3de0af
    • Peter Eisentraut's avatar
      doc: Document that logical replication supports synchronous replication · 2dca0343
      Peter Eisentraut authored
      Update the documentation a bit to include that logical replication as
      well as other and third-party replication clients can participate in
      synchronous replication.
      2dca0343
    • Peter Eisentraut's avatar
      Refine memory allocation in ICU conversions · d8b3c813
      Peter Eisentraut authored
      The simple calculations done to estimate the size of the output buffers
      for ucnv_fromUChars() and ucnv_toUChars() could overflow int32_t for
      large strings.  To avoid that, go the long way and run the function
      first without an output buffer to get the correct output buffer size
      requirement.
      d8b3c813
  5. 01 Jul, 2017 4 commits
    • Tom Lane's avatar
      Clean up misuse and nonuse of poll_query_until(). · b0f069d9
      Tom Lane authored
      Several callers of PostgresNode::poll_query_until() neglected to check
      for failure; I do not think that's optional.  Also, rewrite one place
      that had reinvented poll_query_until() for no very good reason.
      b0f069d9
    • Tom Lane's avatar
      Reduce delay for last logicalrep feedback message when master goes idle. · f32678c0
      Tom Lane authored
      The regression tests contain numerous cases where we do some activity on a
      master server and then wait till the slave has ack'd flushing its copy of
      that transaction.  Because WAL flush on the slave is asynchronous to the
      logicalrep worker process, the worker cannot send such a feedback message
      during the LogicalRepApplyLoop iteration where it processes the last data
      from the master.  In the previous coding, the feedback message would come
      out only when the loop's WaitLatchOrSocket call returned WL_TIMEOUT.  That
      requires one full second of delay (NAPTIME_PER_CYCLE); and to add insult
      to injury, it could take more than that if the WaitLatchOrSocket was
      interrupted a few times by latch-setting events.
      
      In reality we can expect the slave's walwriter process to have flushed the
      WAL data after, more or less, WalWriterDelay (typically 200ms).  Hence,
      if there are unacked transactions pending, make the wait delay only that
      long rather than the full NAPTIME_PER_CYCLE.  Also, move one of the
      send_feedback() calls into the loop main line, so that we'll check for the
      need to send feedback even if we were woken by a latch event and not either
      socket data or timeout.
      
      It's not clear how much this matters for production purposes, but
      it's definitely helpful for testing.
      
      Discussion: https://postgr.es/m/30864.1498861103@sss.pgh.pa.us
      f32678c0
    • Tom Lane's avatar
      Shorten timeouts while waiting for logicalrep worker slot attach/detach. · 799f8bc7
      Tom Lane authored
      When waiting for a logical replication worker process to start or stop,
      we have to busy-wait until we see it add or remove itself from the
      LogicalRepWorker slot in shared memory.  Those loops were using a
      one-second delay between checks, but on any reasonably modern machine, it
      doesn't take more than a couple of msec for a worker to spawn or shut down.
      Reduce the loop delays to 10ms to avoid wasting quite so much time in the
      related regression tests.
      
      In principle, a better solution would be to fix things so that the waiting
      process can be awakened via its latch at the right time.  But that seems
      considerably more invasive, which is undesirable for a post-beta fix.
      Worker start/stop performance likely isn't of huge interest anyway for
      production purposes, so we might not ever get around to it.
      
      In passing, rearrange the second wait loop in logicalrep_worker_stop()
      so that the lock is held at the top of the loop, thus saving one lock
      acquisition/release per call, and making it look more like the other loop.
      
      Discussion: https://postgr.es/m/30864.1498861103@sss.pgh.pa.us
      799f8bc7
    • Peter Eisentraut's avatar
      Fix UPDATE of GENERATED ALWAYS identity columns · ef74e03e
      Peter Eisentraut authored
      The bug would previously prevent the update of any column in a table
      with identity columns, rather than just the actual identity column.
      
      Reported-by: zam6ak@gmail.com
      Bug: #14718
      ef74e03e
  6. 30 Jun, 2017 13 commits
    • Alvaro Herrera's avatar
      Fix locking in WAL receiver/sender shmem state structs · 572d6ee6
      Alvaro Herrera authored
      In WAL receiver and WAL server, some accesses to their corresponding
      shared memory control structs were done without holding any kind of
      lock, which could lead to inconsistent and possibly insecure results.
      
      In walsender, fix by clarifying the locking rules and following them
      correctly, as documented in the new comment in walsender_private.h;
      namely that some members can be read in walsender itself without a lock,
      because the only writes occur in the same process.  The rest of the
      struct requires spinlock for accesses, as usual.
      
      In walreceiver, fix by always holding spinlock while accessing the
      struct.
      
      While there is potentially a problem in all branches, it is minor in
      stable ones.  This only became a real problem in pg10 because of quorum
      commit in synchronous replication (commit 3901fd70), and a potential
      security problem in walreceiver because a superuser() check was removed
      by default monitoring roles (commit 25fff407).  Thus, no backpatch.
      
      In passing, clean up some leftover braces which were used to create
      unconditional blocks.  Once upon a time these were used for
      volatile-izing accesses to those shmem structs, which is no longer
      required.  Many other occurrences of this pattern remain.
      
      Author: Michaël Paquier
      Reported-by: Michaël Paquier
      Reviewed-by: Masahiko Sawada, Kyotaro Horiguchi, Thomas Munro,
      	Robert Haas
      Discussion: https://postgr.es/m/CAB7nPqTWYqtzD=LN_oDaf9r-hAjUEPAy0B9yRkhcsLdRN8fzrw@mail.gmail.com
      572d6ee6
    • Peter Eisentraut's avatar
      PL/Python: Fix hint about returning composite type from Python · 898d24ae
      Peter Eisentraut authored
      ('foo') is not a Python tuple: it is a string wrapped in parentheses.  A
      valid 1-element Python tuple is ('foo',).
      
      Author: Daniele Varrazzo <daniele.varrazzo@gmail.com>
      898d24ae
    • Peter Eisentraut's avatar
      Fix typo in comment · b295cc3b
      Peter Eisentraut authored
      Author: Masahiko Sawada <sawada.mshk@gmail.com>
      b295cc3b
    • Tom Lane's avatar
      Fix race conditions and missed wakeups in syncrep worker signaling. · 1f201a81
      Tom Lane authored
      When a sync worker is waiting for the associated apply worker to notice
      that it's in SYNCWAIT state, wait_for_worker_state_change() would just
      patiently wait for that to happen.  This generally required waiting for
      the 1-second timeout in LogicalRepApplyLoop to elapse.  Kicking the worker
      via its latch makes things significantly snappier.
      
      While at it, fix race conditions that could potentially result in crashes:
      we can *not* call logicalrep_worker_wakeup_ptr() once we've released the
      LogicalRepWorkerLock, because worker->proc might've been reset to NULL
      after we do that (indeed, there's no really solid reason to believe that
      the LogicalRepWorker slot even belongs to the same worker anymore).
      In logicalrep_worker_wakeup(), we can just move the wakeup inside the
      lock scope.  In process_syncing_tables_for_apply(), a bit more code
      rearrangement is needed.
      
      Also improve some nearby comments.
      1f201a81
    • Peter Eisentraut's avatar
      Fix typo in comment · 1db49c3b
      Peter Eisentraut authored
      Author: Albe Laurenz <laurenz.albe@wien.gv.at>
      1db49c3b
    • Peter Eisentraut's avatar
      Fix typo in comment · da8f26ec
      Peter Eisentraut authored
      Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
      da8f26ec
    • Peter Eisentraut's avatar
      Remove outdated comment · 1acc04e4
      Peter Eisentraut authored
      Author: Thomas Munro <thomas.munro@enterprisedb.com>
      1acc04e4
    • Peter Eisentraut's avatar
      Update code comments for pg_xlog -> pg_wal · 4260c05c
      Peter Eisentraut authored
      Author: Michael Paquier <michael.paquier@gmail.com>
      4260c05c
    • Tom Lane's avatar
      Check for error during PQendcopy. · 609fa63d
      Tom Lane authored
      Oversight in commit 78c8c814; noted while nosing around the
      walreceiver startup/shutdown code.
      609fa63d
    • Tom Lane's avatar
      Fix walsender to exit promptly if client requests shutdown. · fca85f8e
      Tom Lane authored
      It's possible for WalSndWaitForWal to be asked to wait for WAL that doesn't
      exist yet.  That's fine, in fact it's the normal situation if we're caught
      up; but when the client requests shutdown we should not keep waiting.
      The previous coding could wait indefinitely if the source server was idle.
      
      In passing, improve the rather weak comments in this area, and slightly
      rearrange some related code for better readability.
      
      Back-patch to 9.4 where this code was introduced.
      
      Discussion: https://postgr.es/m/14154.1498781234@sss.pgh.pa.us
      fca85f8e
    • Peter Eisentraut's avatar
      Prohibit creating ICU collation with different ctype · 13a57710
      Peter Eisentraut authored
      ICU does not support "collate" and "ctype" being different, so the
      collctype catalog column is ignored.  But for catalog neatness, ensure
      that they are the same.
      13a57710
    • Robert Haas's avatar
    • Peter Eisentraut's avatar
      Copy collencoding in CREATE COLLATION / FROM · 54baa481
      Peter Eisentraut authored
      This command used to compute the collencoding entry like when a
      completely new collation is created.  But for example when copying the
      "C" collation, this would then result in a collation that has a
      collencoding entry for the current database encoding rather than -1,
      thus not making an exact copy.  This has probably no practical impact,
      but making this change keeps the catalog contents neat.
      Reported-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      54baa481
  7. 29 Jun, 2017 1 commit
    • Tom Lane's avatar
      Eat XIDs more efficiently in recovery TAP test. · 08aed660
      Tom Lane authored
      The point of this loop is to insert 1000 rows into the test table
      and consume 1000 XIDs.  I can't see any good reason why it's useful
      to launch 1000 psqls and 1000 backend processes to accomplish that.
      Pushing the looping into a plpgsql DO block shaves about 10 seconds
      off the runtime of the src/test/recovery TAP tests on my machine;
      that's over 10% of the runtime of that test suite.
      
      It is, in fact, sufficiently more efficient that we now demonstrably
      need wait_slot_xmins() afterwards, or the slaves' xmins may not have
      moved yet.
      08aed660
  8. 28 Jun, 2017 6 commits
    • Tom Lane's avatar
      Ooops, WIN32 code in pg_ctl.c still needs PQExpBuffer. · 1ae85365
      Tom Lane authored
      Per buildfarm.
      1ae85365
    • Tom Lane's avatar
      Change pg_ctl to detect server-ready by watching status in postmaster.pid. · f13ea95f
      Tom Lane authored
      Traditionally, "pg_ctl start -w" has waited for the server to become
      ready to accept connections by attempting a connection once per second.
      That has the major problem that connection issues (for instance, a
      kernel packet filter blocking traffic) can't be reliably told apart
      from server startup issues, and the minor problem that if server startup
      isn't quick, we accumulate "the database system is starting up" spam
      in the server log.  We've hacked around many of the possible connection
      issues, but it resulted in ugly and complicated code in pg_ctl.c.
      
      In commit c61559ec, I changed the probe rate to every tenth of a second.
      That prompted Jeff Janes to complain that the log-spam problem had become
      much worse.  In the ensuing discussion, Andres Freund pointed out that
      we could dispense with connection attempts altogether if the postmaster
      were changed to report its status in postmaster.pid, which "pg_ctl start"
      already relies on being able to read.  This patch implements that, teaching
      postmaster.c to report a status string into the pidfile at the same
      state-change points already identified as being of interest for systemd
      status reporting (cf commit 7d17e683).  pg_ctl no longer needs to link
      with libpq at all; all its functions now depend on reading server files.
      
      In support of this, teach AddToDataDirLockFile() to allow addition of
      postmaster.pid lines in not-necessarily-sequential order.  This is needed
      on Windows where the SHMEM_KEY line will never be written at all.  We still
      have the restriction that we don't want to truncate the pidfile; document
      the reasons for that a bit better.
      
      Also, fix the pg_ctl TAP tests so they'll notice if "start -w" mode
      is broken --- before, they'd just wait out the sixty seconds until
      the loop gives up, and then report success anyway.  (Yes, I found that
      out the hard way.)
      
      While at it, arrange for pg_ctl to not need to #include miscadmin.h;
      as a rather low-level backend header, requiring that to be compilable
      client-side is pretty dubious.  This requires moving the #define's
      associated with the pidfile into a new header file, and moving
      PG_BACKEND_VERSIONSTR someplace else.  For lack of a clearly better
      "someplace else", I put it into port.h, beside the declaration of
      find_other_exec(), since most users of that macro are passing the value to
      find_other_exec().  (initdb still depends on miscadmin.h, but at least
      pg_ctl and pg_upgrade no longer do.)
      
      In passing, fix main.c so that PG_BACKEND_VERSIONSTR actually defines the
      output of "postgres -V", which remarkably it had never done before.
      
      Discussion: https://postgr.es/m/CAMkU=1xJW8e+CTotojOMBd-yzUvD0e_JZu2xHo=MnuZ4__m7Pg@mail.gmail.com
      f13ea95f
    • Andrew Gierth's avatar
      Fix transition tables for ON CONFLICT. · 8c55244a
      Andrew Gierth authored
      We now disallow having triggers with both transition tables and ON
      INSERT OR UPDATE (which was a PG extension to the spec anyway),
      because in this case it's not at all clear how the transition tables
      should work for an INSERT ... ON CONFLICT query.  Separate ON INSERT
      and ON UPDATE triggers with transition tables are allowed, and the
      transition tables for these reflect only the inserted and only the
      updated tuples respectively.
      
      Patch by Thomas Munro
      
      Discussion: https://postgr.es/m/CAEepm%3D11KHQ0JmETJQihSvhZB5mUZL2xrqHeXbCeLhDiqQ39%3Dw%40mail.gmail.com
      8c55244a
    • Andrew Gierth's avatar
      Fix transition tables for wCTEs. · c46c0e52
      Andrew Gierth authored
      The original coding didn't handle this case properly; each separate
      DML substatement needs its own set of transitions.
      
      Patch by Thomas Munro
      
      Discussion: https://postgr.es/m/CAL9smLCDQ%3D2o024rBgtD4WihzX8B3C6u_oSQ2K3%2BR5grJrV0bg%40mail.gmail.com
      c46c0e52
    • Andrew Gierth's avatar
      Fix transition tables for partition/inheritance. · 501ed02c
      Andrew Gierth authored
      We disallow row-level triggers with transition tables on child tables.
      Transition tables for triggers on the parent table contain only those
      columns present in the parent.  (We can't mix tuple formats in a
      single transition table.)
      
      Patch by Thomas Munro
      
      Discussion: https://postgr.es/m/CA%2BTgmoZzTBBAsEUh4MazAN7ga%3D8SsMC-Knp-6cetts9yNZUCcg%40mail.gmail.com
      501ed02c
    • Tom Lane's avatar
      Second try at fixing tcp_keepalives_idle option on Solaris. · 99255d73
      Tom Lane authored
      Buildfarm evidence shows that TCP_KEEPALIVE_THRESHOLD doesn't exist
      after all on Solaris < 11.  This means we need to take positive action to
      prevent the TCP_KEEPALIVE code path from being taken on that platform.
      I've chosen to limit it with "&& defined(__darwin__)", since it's unclear
      that anyone else would follow Apple's precedent of spelling the symbol
      that way.
      
      Also, follow a suggestion from Michael Paquier of eliminating code
      duplication by defining a couple of intermediate symbols for the
      socket option.
      
      In passing, make some effort to reduce the number of translatable messages
      by replacing "setsockopt(foo) failed" with "setsockopt(%s) failed", etc,
      throughout the affected files.  And update relevant documentation so
      that it doesn't claim to provide an exhaustive list of the possible
      socket option names.
      
      Like the previous commit (f0256c77), back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/20170627163757.25161.528@wrigleys.postgresql.org
      99255d73