1. 09 Jan, 2018 8 commits
    • Tom Lane's avatar
      While waiting for a condition variable, detect postmaster death. · 80259d4d
      Tom Lane authored
      The general assumption for postmaster child processes is that they
      should just exit(1), reasonably promptly, if the postmaster disappears.
      condition_variable.c neglected this consideration and could be left
      waiting forever, if the counterpart process it is waiting for has
      done the right thing and exited.
      
      We had some discussion of adjusting the WaitEventSet API to make it
      harder to make this type of mistake in future; but for the moment,
      and for v10, let's make this narrow fix.
      
      Discussion: https://postgr.es/m/20412.1515456143@sss.pgh.pa.us
      80259d4d
    • Peter Eisentraut's avatar
      Fix ssl tests for when tls-server-end-point is not supported · c3d41ccf
      Peter Eisentraut authored
      Add a function to TestLib that allows us to check pg_config.h and then
      decide the expected test outcome based on that.
      
      Author: Michael Paquier <michael.paquier@gmail.com>
      c3d41ccf
    • Tom Lane's avatar
      Fix race condition during replication origin drop. · 8a906204
      Tom Lane authored
      replorigin_drop() misunderstood the API for condition variables: it
      had ConditionVariablePrepareToSleep and ConditionVariableCancelSleep
      inside its test-and-sleep loop, rather than outside the loop as
      intended.  The net effect is a narrow race-condition window wherein,
      if the process using a replication slot releases it immediately after
      replorigin_drop() releases the ReplicationOriginLock, replorigin_drop()
      would get into the condition variable's wait list too late and then
      wait indefinitely for a signal that won't come.
      
      Because there's a different CV for each replication slot, we can't
      just move the ConditionVariablePrepareToSleep call to above the
      test-and-sleep loop.  What we can do, in the wake of commit 13db3b93,
      is drop the ConditionVariablePrepareToSleep call entirely.  This fix
      depends on that commit because (at least in principle) the slot matching
      the target replication origin might move around, so that once in a blue
      moon successive loop iterations might involve different CVs.  We can now
      cope with such a scenario, at the cost of an extra trip through the
      retry loop.
      
      (There are ways we could fix this bug without depending on that commit,
      but they're all a lot more complicated than this way.)
      
      While at it, upgrade the rather skimpy comments in this function.
      
      Back-patch to v10 where this code came in.
      
      Discussion: https://postgr.es/m/19947.1515455433@sss.pgh.pa.us
      8a906204
    • Tom Lane's avatar
      Allow ConditionVariable[PrepareTo]Sleep to auto-switch between CVs. · 13db3b93
      Tom Lane authored
      The original coding here insisted that callers manually cancel any prepared
      sleep for one condition variable before starting a sleep on another one.
      While that's not a huge burden today, it seems like a gotcha that will bite
      us in future if the use of condition variables increases; anything we can
      do to make the use of this API simpler and more robust is attractive.
      Hence, allow these functions to automatically switch their attention to
      a different CV when required.  This is safe for the same reason it was OK
      for commit aced5a92 to let a broadcast operation cancel any prepared CV
      sleep: whenever we return to the other test-and-sleep loop, we will
      automatically re-prepare that CV, paying at most an extra test of that
      loop's exit condition.
      
      Back-patch to v10 where condition variables were introduced.  Ordinarily
      we would probably not back-patch a change like this, but since it does not
      invalidate any coding pattern that was legal before, it seems safe enough.
      Furthermore, there's an open bug in replorigin_drop() for which the
      simplest fix requires this.  Even if we chose to fix that in some more
      complicated way, the hazard would remain that we might back-patch some
      other bug fix that requires this behavior.
      
      Patch by me, reviewed by Thomas Munro.
      
      Discussion: https://postgr.es/m/2437.1515368316@sss.pgh.pa.us
      13db3b93
    • Robert Haas's avatar
      Don't allow VACUUM VERBOSE ANALYZE VERBOSE. · 921059bd
      Robert Haas authored
      There are plans to extend the syntax for ANALYZE, so we need to break
      the link between VacuumStmt and AnalyzeStmt.  But apart from that, the
      syntax above is undocumented and, if discovered by users, might give
      the impression that the VERBOSE option for VACUUM differs from the
      verbose option from ANALYZE, which it does not.
      
      Nathan Bossart, reviewed by Michael Paquier and Masahiko Sawada
      
      Discussion: http://postgr.es/m/D3FC73E2-9B1A-4DB4-8180-55F57D116B4E@amazon.com
      921059bd
    • Teodor Sigaev's avatar
      Improve scripting language in pgbench · bc7fa0c1
      Teodor Sigaev authored
      Added:
       - variable now might contain integer, double, boolean and null values
       - functions ln, exp
       - logical AND/OR/NOT
       - bitwise AND/OR/NOT/XOR
       - bit right/left shift
       - comparison operators
       - IS [NOT] (NULL|TRUE|FALSE)
       - conditional choice (in form of when/case/then)
      
      New operations and functions allow to implement more complicated test scenario.
      
      Author: Fabien Coelho with minor editorization by me
      Reviewed-By: Pavel Stehule, Jeevan Ladhe, me
      Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.10.1604030742390.31618@sto
      bc7fa0c1
    • Robert Haas's avatar
      Fix comment. · 63008b19
      Robert Haas authored
      RELATION_IS_OTHER_TEMP is tested in the caller, not here.
      
      Discussion: http://postgr.es/m/5A5438E4.3090709@lab.ntt.co.jp
      63008b19
    • Bruce Momjian's avatar
      pg_upgrade: prevent check on live cluster from generating error · d25ee300
      Bruce Momjian authored
      Previously an inaccurate but harmless error was generated when running
      --check on a live server before reporting the servers as compatible.
      The fix is to split error reporting and exit control in the exec_prog()
      API.
      
      Reported-by: Daniel Westermann
      
      Backpatch-through: 10
      d25ee300
  2. 08 Jan, 2018 3 commits
    • Tom Lane's avatar
    • Tom Lane's avatar
      Improve error detection capability in proclists. · ea8e1bbc
      Tom Lane authored
      Previously, although the initial state of a proclist_node is expected
      to be next == prev == 0, proclist_delete_offset would reset nodes to
      next == prev == INVALID_PGPROCNO when removing them from a list.
      This is the same state that a node in a singleton list has, so that
      it's impossible to distinguish not-in-a-list from in-a-list.  Change
      proclist_delete_offset to reset removed nodes to next == prev == 0,
      making it possible to distinguish those cases, and then add Asserts
      to the list add and delete functions that the supplied node isn't
      or is in a list at entry.  Also tighten assertions about the node
      being in the particular list (not some other one) where it is possible
      to check that in O(1) time.
      
      In ConditionVariablePrepareToSleep, since we don't expect the process's
      cvWaitLink to already be in a list, remove the more-or-less-useless
      proclist_contains check; we'd rather have proclist_push_tail's new
      assertion fire if that happens.
      
      Improve various comments related to proclists, too.
      
      Patch by me, reviewed by Thomas Munro.  This isn't back-patchable, since
      there could theoretically be inlined copies of proclist_delete_offset in
      third-party modules.  But it's only improving debuggability anyway.
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      ea8e1bbc
    • Tom Lane's avatar
      Back off chattiness in RemovePgTempFiles(). · eeb3c2df
      Tom Lane authored
      In commit 561885db, as part of normalizing RemovePgTempFiles's error
      handling, I removed its behavior of silently ignoring ENOENT failures
      during directory opens.  Thomas Munro points out that this is a bad idea at
      the top level, because we don't create pgsql_tmp directories until needed.
      Thus this coding could produce LOG messages in perfectly normal situations,
      which isn't what I intended.  Restore the suppression of ENOENT logging,
      but only at top level --- it would still be unexpected for a nested temp
      directory to disappear between seeing it in the parent directory and
      opening it.
      
      Discussion: https://postgr.es/m/CAEepm=2y06SehAkTnd5sU_eVqdv5P-=Srt1y5vYNQk6yVDVaPw@mail.gmail.com
      eeb3c2df
  3. 06 Jan, 2018 5 commits
    • Simon Riggs's avatar
      Add TIMELINE to backup_label file · 6271fceb
      Simon Riggs authored
      Allows new test to confirm timelines match
      
      Author: Michael Paquier
      Reviewed-by: David Steele
      6271fceb
    • Simon Riggs's avatar
      Default monitoring roles - errata · 6668a54e
      Simon Riggs authored
      25fff407 introduced
      default monitoring roles. Apply these corrections:
      
      * Allow access to pg_stat_get_wal_senders()
        by role pg_read_all_stats
      
      * Correct comment in pg_stat_get_wal_receiver()
        to show it is no longer superuser-only.
      
      Author: Feike Steenbergen
      Reviewed-by: Michael Paquier
      
      Apply to HEAD, then later backpatch to 10
      6668a54e
    • Tom Lane's avatar
      Remove return values of ConditionVariableSignal/Broadcast. · ccf312a4
      Tom Lane authored
      In the wake of commit aced5a92, the semantics of these results are
      a bit squishy: we can tell whether we signaled some other process(es),
      but we do not know which ones were real waiters versus mere sentinels
      for ConditionVariableBroadcast operations.  It does not help much that
      ConditionVariableBroadcast will attempt to pass on the signal to the
      next real waiter, because (a) there might not be one, and (b) that will
      only happen awhile later, anyway.  So these results could overstate how
      much effect the calls really had.
      
      However, no existing caller of either function pays any attention to its
      result value, so it seems reasonable to just define that as a required
      property of a correct algorithm.  To encourage correctness and save some
      tiny number of cycles, change both functions to return void.
      
      Patch by me, per an observation by Thomas Munro.  No back-patch, since
      if any third parties happen to be using these functions, they might not
      appreciate an API break in a minor release.
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      ccf312a4
    • Tom Lane's avatar
      Reorder steps in ConditionVariablePrepareToSleep for more safety. · 3cac0ec8
      Tom Lane authored
      In the admittedly-very-unlikely case that AddWaitEventToSet fails,
      ConditionVariablePrepareToSleep would error out after already having
      set cv_sleep_target, which is probably bad, and after having already
      set cv_wait_event_set, which is very bad.  Transaction abort might or
      might not clean up cv_sleep_target properly; but there is nothing
      that would be aware that the WaitEventSet wasn't fully constructed,
      so that all future condition variable sleeps would be broken.
      We can easily guard against these hazards with slight restructuring.
      
      Back-patch to v10 where condition_variable.c was introduced.
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      3cac0ec8
    • Tom Lane's avatar
      Rewrite ConditionVariableBroadcast() to avoid live-lock. · aced5a92
      Tom Lane authored
      The original implementation of ConditionVariableBroadcast was, per its
      self-description, "the dumbest way possible".  Thomas Munro found out
      it was a bit too dumb.  An awakened process may immediately re-queue
      itself, if the specific condition it's waiting for is not yet satisfied.
      If this happens before ConditionVariableBroadcast is able to see the wait
      queue as empty, then ConditionVariableBroadcast will re-awaken the same
      process, repeating the cycle.  Given unlucky timing this back-and-forth
      can repeat indefinitely; loops lasting thousands of seconds have been
      seen in testing.
      
      To fix, add our own process to the end of the wait queue to serve as a
      sentinel, and exit the broadcast loop once our process is not there
      anymore.  There are various special considerations described in the
      comments, the principal disadvantage being that wakers can no longer
      be sure whether they awakened a real waiter or just a sentinel.  But in
      practice nobody pays attention to the result of ConditionVariableSignal
      or ConditionVariableBroadcast anyway, so that problem seems hypothetical.
      
      Back-patch to v10 where condition_variable.c was introduced.
      
      Tom Lane and Thomas Munro
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      aced5a92
  4. 05 Jan, 2018 6 commits
  5. 04 Jan, 2018 11 commits
  6. 03 Jan, 2018 7 commits