1. 29 Apr, 2016 7 commits
    • Kevin Grittner's avatar
      Add a few entries to the tail of time mapping, to see old values. · 7c3e8039
      Kevin Grittner authored
      Without a few entries beyond old_snapshot_threshold, the lookup
      would often fail, resulting in the more aggressive pruning or
      vacuum being skipped often enough to matter.  This was very clearly
      shown by a python test script posted by Ants Aasma, and was likely
      a factor in an earlier but somewhat less clear-cut test case posted
      by Jeff Janes.
      
      This patch makes no change to the logic, per se -- it just makes
      the array of mapping entries big enough to make lookup misses based
      on timing much less likely.  An occasional miss is still possible
      if a thread stalls for more than 10 minutes, but that does not
      create any problem with correctness of behavior.  Besides, if
      things are so busy that a thread is stalling for more than 10
      minutes, it is probably OK to skip the more aggressive cleanup at
      that particular point in time.
      7c3e8039
    • Andrew Dunstan's avatar
      Fix comment whitespace in VS2105 patch · d34e7b28
      Andrew Dunstan authored
      per gripe from Michael Paquier.
      d34e7b28
    • Peter Eisentraut's avatar
      doc: Minor wording changes · 82881b2b
      Peter Eisentraut authored
      From: Dmitry Igrishin <dmitigr@gmail.com>
      82881b2b
    • Magnus Hagander's avatar
      Fix typo · a03bda32
      Magnus Hagander authored
      Author: Thomas Munro
      a03bda32
    • Andrew Dunstan's avatar
      Fix typo in VS2015 patch · 7dc54923
      Andrew Dunstan authored
      reported by Christian Ullrich
      7dc54923
    • Andrew Dunstan's avatar
      Support building with Visual Studio 2015 · 0fb54de9
      Andrew Dunstan authored
      Adjust the way we detect the locale. As a result the minumum Windows
      version supported by VS2015 and later is Windows Vista. Add some tweaks
      to remove new compiler warnings. Remove documentation references to the
      now obsolete msysGit.
      
      Michael Paquier, somewhat edited by me, reviewed by Christian Ullrich.
      
      Backpatch to 9.5
      0fb54de9
    • Andres Freund's avatar
      Remember asking for feedback during walsender shutdown. · 59455018
      Andres Freund authored
      Since 5a991ef8 we're explicitly asking for feedback from the receiving
      side when shutting down walsender, if there's not yet replicated
      data.
      
      Unfortunately we didn't remember (i.e. set waiting_for_ping_response to
      true) having asked for feedback, leading to scenarios in which replies
      were requested at a high frequency.
      
      I can't reproduce this problem on my laptop, I think that's because the
      problem requires a significant TCP window to manifest due to the
      !pq_is_send_pending() condition. But since this clearly is a bug, let's
      fix it.  There's quite possibly more wrong than just this though.
      
      While fiddling with WalSndDone(), I rewrote a hard to understand comment
      about looking at the flush vs. the write position.
      
      Reported-By: Nick Cleaton, Magnus Hagander
      Author: Nick Cleaton
      Discussion: CAFgz3kus=rC_avEgBV=+hRK5HYJ8vXskJRh8yEAbahJGTzF2VQ@mail.gmail.com
          CABUevExsjROqDcD0A2rnJ6HK6FuKGyewJr3PL12pw85BHFGS2Q@mail.gmail.com
      Backpatch: 9.4, were 5a991ef8 introduced the use of feedback messages
          during shutdown.
      59455018
  2. 28 Apr, 2016 4 commits
    • Tom Lane's avatar
      Adjust DatumGetBool macro, this time for sure. · 23b09e15
      Tom Lane authored
      Commit 23a41573 attempted to fix the DatumGetBool macro to ignore bits
      in a Datum that are to the left of the actual bool value.  But it did that
      by casting the Datum to bool; and on compilers that use C99 semantics for
      bool, that ends up being a whole-word test, not a 1-byte test.  This seems
      to be the true explanation for contrib/seg failing in VS2015.  To fix, use
      GET_1_BYTE() explicitly.  I think in the previous patch, I'd had some idea
      of not having to commit to bool being exactly 1 byte wide, but regardless
      of what the compiler's bool is, boolean columns and Datums are certainly
      1 byte wide.
      
      The previous fix was (eventually) back-patched into all active versions,
      so do likewise with this one.
      23b09e15
    • Tom Lane's avatar
      Revert "Convert contrib/seg's bool-returning SQL functions to V1 call convention." · f0504230
      Tom Lane authored
      This reverts commit c8e81afc.
      That turns out to have been based on a faulty diagnosis of why the
      VS2015 build was misbehaving.  Instead, we need to fix DatumGetBool().
      f0504230
    • Teodor Sigaev's avatar
      Prevent to use magic constants · f8467f7d
      Teodor Sigaev authored
      Use macroses for definition amstrategies/amsupport fields instead of
      hardcoded values.
      
      Author: Nikolay Shaplov with addition for contrib/bloom
      f8467f7d
    • Teodor Sigaev's avatar
      Prevent multiple cleanup process for pending list in GIN. · e2c79e14
      Teodor Sigaev authored
      Previously, ginInsertCleanup could exit early if it detects that someone else
      is cleaning up the pending list, without waiting for that someone else to
      finish the job. But in this case vacuum could miss tuples to be deleted.
      
      Cleanup process now locks metapage with a help of heavyweight
      LockPage(ExclusiveLock), and it guarantees that there is no another cleanup
      process at the same time. Lock is taken differently depending on caller of
      cleanup process: any vacuums and gin_clean_pending_list() will be blocked
      until lock becomes available, ordinary insert uses conditional lock to
      prevent indefinite waiting on lock.
      
      Insert into pending list doesn't use this lock, so insertion isn't blocked.
      
      Also, patch adds stopping of cleanup process when at-start-cleanup-tail is
      reached in order to prevent infinite cleanup in case of massive insertion. But
      it will stop only for automatic maintenance tasks like autovacuum.
      
      Patch introduces choice of limit of memory to use: autovacuum_work_mem,
      maintenance_work_mem or work_mem depending on call path.
      
      Patch for previous releases should be reworked due to changes between 9.6 and
      previous ones in this area.
      
      Discover and diagnostics by Jeff Janes and Tomas Vondra
      
      Patch by me with some ideas of Jeff Janes
      e2c79e14
  3. 27 Apr, 2016 15 commits
    • Tom Lane's avatar
      Use memmove() not memcpy() to slide some pointers down. · ad520ec4
      Tom Lane authored
      The previous coding here was formally undefined, though it seems to
      accidentally work on most platforms in the buildfarm.  Caught by some
      OpenBSD platforms in which libc contains an assertion check for
      overlapping areas passed to memcpy().
      
      Thomas Munro
      ad520ec4
    • Tom Lane's avatar
      Clean up parsing of synchronous_standby_names GUC variable. · 4c804fbd
      Tom Lane authored
      Commit 989be081 added a flex/bison lexer/parser to interpret
      synchronous_standby_names.  It was done in a pretty crufty way, though,
      making assorted end-use sites responsible for calling the parser at the
      right times.  That was not only vulnerable to errors of omission, but made
      it possible that lexer/parser errors occur at very undesirable times,
      and created memory leakages even if there was no error.
      
      Instead, perform the parsing once during check_synchronous_standby_names
      and let guc.c manage the resulting data.  To do that, we have to flatten
      the parsed representation into a single hunk of malloc'd memory, but that
      is not very hard.
      
      While at it, work a little harder on making useful error reports for
      parsing problems; the previous code felt that "synchronous_standby_names
      parser returned 1" was an appropriate user-facing error message.  (To
      be fair, it did also log a syntax error message, but separately from the
      GUC problem report, which is at best confusing.)  It had some outright
      bugs in the face of invalid input, too.
      
      I (tgl) also concluded that we need to restrict unquoted names in
      synchronous_standby_names to be just SQL identifiers.  The previous coding
      would accept darn near anything, which (1) makes the quoting convention
      both nearly-unnecessary and formally ambiguous, (2) makes it very hard to
      understand what is a syntax error and what is a creative interpretation of
      the input as a standby name, and (3) makes it impossible to further extend
      the syntax in future without a compatibility break.  I presume that we're
      intending future extensions of the syntax, else this parsing infrastructure
      is massive overkill, so (3) is an important objection.  Since we've taken
      a compatibility hit for non-identifier names with this change anyway, we
      might as well lock things down now and insist that users use double quotes
      for standby names that aren't identifiers.
      
      Kyotaro Horiguchi and Tom Lane
      4c804fbd
    • Robert Haas's avatar
      Fix wrong word. · 372ff7ca
      Robert Haas authored
      Commit a31212b4 was a little too hasty.
      
      Per report from Tom Lane.
      372ff7ca
    • Robert Haas's avatar
      Change postgresql.conf.sample to say that fsync=off will corrupt data. · a31212b4
      Robert Haas authored
      Discussion: 24748.1461764666@sss.pgh.pa.us
      
      Per a suggestion from Craig Ringer.  This wording from Tom Lane,
      following discussion.
      a31212b4
    • Robert Haas's avatar
      Tighten up sanity checks for parallel aggregate in execQual.c. · cf402ba7
      Robert Haas authored
      David Rowley
      cf402ba7
    • Robert Haas's avatar
      Remove inadvertently commited vim swapfile. · b33dc776
      Robert Haas authored
      If you were wondering what editor I use, now you know.
      b33dc776
    • Robert Haas's avatar
      Update typedefs.list file in preparation for pgindent run · acb51bd7
      Robert Haas authored
      In addition to adding new typedefs, I also re-sorted the file so that
      various entries add piecemeal, mostly or entirely by me, were alphabetized
      the same way as other entries in the file.
      acb51bd7
    • Robert Haas's avatar
      Clean up a few parallelism-related things that pgindent wants to mangle. · 8126eaee
      Robert Haas authored
      In nodeFuncs.c, pgindent wants to introduce spurious indentation into
      the definitions of planstate_tree_walker and planstate_walk_subplans.
      Fix that by spreading the definition out across several lines, similar
      to what is already done for other walker functions in that file.
      
      In execParallel.c, in the definition of SharedExecutorInstrumentation,
      pgindent wants to insert more whitespace between the type name and the
      member name.  That causes it to mangle comments later on the line.  Fix
      by moving the comments out of line.  Now that we have a bit more room,
      add some more details that may be useful to the next person reading
      this code.
      8126eaee
    • Robert Haas's avatar
      Remove mergeHyperLogLog. · 360ca27a
      Robert Haas authored
      It's buggy.  If somebody needs this later, they'll need to put back
      a non-buggy vesion of it.
      
      Discussion: CAM3SWZT-i6R9JU5YXa8MJUou2_r3LfGJZpQ9tYa1BYxfkj0=cQ@mail.gmail.com
      Discussion: CAM3SWZRUOLsYoTT83QgdUy9D8ehYWm_nvbrrfcOOzikiRfFY7g@mail.gmail.com
      
      Peter Geoghegan
      360ca27a
    • Robert Haas's avatar
      Fix EXPLAIN VERBOSE output for parallel aggregate. · 59eb5512
      Robert Haas authored
      The way that PartialAggregate and FinalizeAggregate plan nodes were
      displaying output columns before was bogus.  Now, FinalizeAggregate
      produces the same outputs as an Aggregate would have produced, while
      PartialAggregate produces each of those outputs prefixed by the word
      PARTIAL.
      
      Discussion: 12585.1460737650@sss.pgh.pa.us
      
      Patch by me, reviewed by David Rowley.
      59eb5512
    • Andres Freund's avatar
      Don't open formally non-existent segments in _mdfd_getseg(). · 72a98a63
      Andres Freund authored
      Before this commit _mdfd_getseg(), in contrast to mdnblocks(), did not
      verify whether all segments leading up to the to-be-opened one, were
      RELSEG_SIZE sized. That is e.g. not the case after truncating a
      relation, because later segments just get truncated to zero length, not
      removed.
      
      Once a "non-existent" segment has been opened in a session, mdnblocks()
      will return wrong results, causing errors like "could not read block %u
      in file" when accessing blocks. Closing the session, or the later
      arrival of relevant invalidation messages, would "fix" the problem.
      
      That, so far, was mostly harmless, because most segment accesses are
      only done after an mdnblocks() call. But since 428b1d6b we try to
      open segments that might have been deleted, to trigger kernel writeback
      from a backend's queue of recent writes.
      
      To fix check segment sizes in _mdfd_getseg() when opening previously
      unopened segments. In practice this shouldn't imply a lot of additional
      lseek() calls, because mdnblocks() will most of the time already have
      opened all relevant segments.
      
      This commit also fixes a second problem, namely that _mdfd_getseg(
      EXTENSION_RETURN_NULL) extends files during recovery, which is not
      desirable for the mdwriteback() case.  Add EXTENSION_REALLY_RETURN_NULL,
      which does not behave that way, and use it.
      
      Reported-By: Thom Brown
      Author: Andres Freund, Abhijit Menon-Sen
      Reviewd-By: Robert Haas, Fabien Coehlo
      Discussion: CAA-aLv6Dp_ZsV-44QA-2zgkqWKQq=GedBX2dRSrWpxqovXK=Pg@mail.gmail.com
      Fixes: 428b1d6b
      72a98a63
    • Andres Freund's avatar
      Emit invalidations to standby for transactions without xid. · c6ff84b0
      Andres Freund authored
      So far, when a transaction with pending invalidations, but without an
      assigned xid, committed, we simply ignored those invalidation
      messages. That's problematic, because those are actually sent for a
      reason.
      
      Known symptoms of this include that existing sessions on a hot-standby
      replica sometimes fail to notice new concurrently built indexes and
      visibility map updates.
      
      The solution is to WAL log such invalidations in transactions without an
      xid. We considered to alternatively force-assign an xid, but that'd be
      problematic for vacuum, which might be run in systems with few xids.
      
      Important: This adds a new WAL record, but as the patch has to be
      back-patched, we can't bump the WAL page magic. This means that standbys
      have to be updated before primaries; otherwise
      "PANIC: standby_redo: unknown op code 32" errors can be encountered.
      
      XXX:
      
      Reported-By: Васильев Дмитрий, Masahiko Sawada
      Discussion:
          CAB-SwXY6oH=9twBkXJtgR4UC1NqT-vpYAtxCseME62ADwyK5OA@mail.gmail.com
          CAD21AoDpZ6Xjg=gFrGPnSn4oTRRcwK1EBrWCq9OqOHuAcMMC=w@mail.gmail.com
      c6ff84b0
    • Robert Haas's avatar
      Fix pg_get_functiondef to dump parallel-safety markings. · 2ac3be2e
      Robert Haas authored
      Ashutosh Sharma
      2ac3be2e
    • Noah Misch's avatar
      Impose a full barrier in generic-xlc.h atomics functions. · 213c7df0
      Noah Misch authored
      pg_atomic_compare_exchange_*_impl() were providing only the semantics of
      an acquire barrier.  Buildfarm members hornet and mandrill revealed this
      deficit beginning with commit 008608b9.
      While we have no report of symptoms in 9.5, we can't rule out the
      possibility of certain compilers, hardware, or extension code relying on
      these functions' specified barrier semantics.  Back-patch to 9.5, where
      commit b64d92f1 introduced atomics.
      
      Reviewed by Andres Freund.
      213c7df0
    • Peter Eisentraut's avatar
      pg_dump: Message style improvements · 3019f432
      Peter Eisentraut authored
      forgotten in b6dacc17
      3019f432
  4. 26 Apr, 2016 6 commits
    • Tom Lane's avatar
      Add a --brief option to git_changelog. · 8067c8f8
      Tom Lane authored
      In commit c0b05019, Andres introduced the idea of including one-line
      commit references in our major release notes.  Teach git_changelog to
      emit a (lightly adapted) version of that format, so that we don't
      have to laboriously add it to the notes after the fact.  The default
      output isn't changed, since I anticipate still using that for minor
      release notes.
      8067c8f8
    • Teodor Sigaev's avatar
      Fix tsearch docs · f1e3c760
      Teodor Sigaev authored
      Remove mention of setweight(tsquery) which wasn't included in 9.6. Also
      replace old forgotten phrase operator to new one.
      
      Dmitry Ivanov
      f1e3c760
    • Tom Lane's avatar
      Fix order of shutdown cleanup operations in PostgresNode.pm. · 08af9219
      Tom Lane authored
      Previously, database clusters created by a TAP test were shut down by
      DESTROY methods attached to the PostgresNode objects representing them.
      The trouble with that is that if the objects survive into the final global
      destruction phase (which they do), Perl executes the DESTROY methods in an
      unspecified order.  Thus, the order of shutdown of multiple clusters was
      indeterminate, which might lead to not-very-reproducible errors getting
      logged (eg from a slave whose master might or might not get killed first).
      Worse, the File::Temp objects representing the temporary PGDATA directories
      might get destroyed before the PostgresNode objects, resulting in attempts
      to delete PGDATA directories that still have live servers in them.  On
      Windows, this would lead to directory deletion failures; on Unix, it
      usually had no effects worse than erratic "could not open temporary
      statistics file "pg_stat/global.tmp": No such file or directory" log
      messages.
      
      While none of this would affect the reported result of the TAP test, which
      is already determined, it could be very confusing when one is trying to
      understand from the logs what went wrong with a failed test.
      
      To fix, do the postmaster shutdowns in an END block rather than at object
      destruction time.  The END block will execute at a well-defined (and
      reasonable) time during script termination, and it will stop the
      postmasters in order of PostgresNode object creation.  (Perhaps we should
      change that to be reverse order of creation, but the main point here is
      that we now have control which we did not before.)  Use "pg_ctl stop", not
      an asynchronous kill(SIGQUIT), so that we wait for the postmasters to shut
      down before proceeding with directory deletion.
      
      Deletion of temporary directories still happens in an unspecified order
      during global destruction, but I can see no reason to care about that
      once the postmasters are stopped.
      08af9219
    • Tom Lane's avatar
      Yet more portability hacking for degree-based trig functions. · 82311bcd
      Tom Lane authored
      The true explanation for Peter Eisentraut's report of inexact asind results
      seems to be that (a) he's compiling into x87 instruction set, which uses
      wider-than-double float registers, plus (b) the library function asin() on
      his platform returns a result that is wider than double and is not rounded
      to double width.  To fix, we have to force the function's result to be
      rounded comparably to what happened to the scaling constant asin_0_5.
      Experimentation suggests that storing it into a volatile local variable is
      the least ugly way of making that happen.  Although only asin() is known to
      exhibit an observable inexact result, we'd better do this in all the places
      where we're hoping to get an exact result by scaling.
      82311bcd
    • Robert Haas's avatar
      Enable parallel query by default. · 77cd477c
      Robert Haas authored
      Change max_parallel_degree default from 0 to 2.  It is possible that
      this is not a good idea, or that we should go with 1 worker rather
      than 2, but we won't find out without trying it.  Along the way,
      reword the documentation for max_parallel_degree a little bit to
      hopefully make it more clear.
      
      Discussion: 20160420174631.3qjjhpwsvvx5bau5@alap3.anarazel.de
      77cd477c
    • Magnus Hagander's avatar
      Fix typo in comment · b7351ced
      Magnus Hagander authored
      Author: Daniel Gustafsson
      b7351ced
  5. 25 Apr, 2016 6 commits
    • Peter Eisentraut's avatar
      pg_dump: Message style improvements · b6dacc17
      Peter Eisentraut authored
      b6dacc17
    • Kevin Grittner's avatar
      Fix C comment typo and redundant test · e65953be
      Kevin Grittner authored
      e65953be
    • Tom Lane's avatar
      New method for preventing compile-time calculation of degree constants. · 6b1a213b
      Tom Lane authored
      Commit 65abaab5 tried to prevent the scaling constants used in
      the degree-based trig functions from being precomputed at compile time,
      because some compilers do that with functions that don't yield results
      identical-to-the-last-bit to what you get at runtime.  A report from
      Peter Eisentraut suggests that some recent compilers are smart enough
      to see through that trick, though.  Instead, let's put the inputs to
      these calculations into non-const global variables, which should be a
      more reliable way of convincing the compiler that it can't assume that
      they are compile-time constants.  (If we really get desperate, we could
      mark these variables "volatile", but I do not believe we should have to.)
      6b1a213b
    • Tom Lane's avatar
      Try harder to detect a port conflict in PostgresNode.pm. · 40e89e2a
      Tom Lane authored
      Commit fab84c77 tried to get away without doing an actual bind(),
      but buildfarm results show that that doesn't get the job done.  So we must
      really bind to the target port --- and at least on my Linux box, we need a
      listen() as well, or conflicts won't be detected.  We rely on SO_REUSEADDR
      to prevent problems from starting a postmaster on the socket immediately
      after we've bound to it in the test code.  (There may be platforms where
      that doesn't work too well.  But fortunately, we only really care whether
      this works on Windows, and there the default behavior should be OK.)
      40e89e2a
    • Peter Eisentraut's avatar
    • Peter Eisentraut's avatar
      doc: Fix typo · 96687497
      Peter Eisentraut authored
      From: Andreas Seltenreich <andreas.seltenreich@credativ.de>
      96687497
  6. 24 Apr, 2016 2 commits
    • Tom Lane's avatar
      Improve PostgresNode.pm's logic for detecting already-in-use ports. · fab84c77
      Tom Lane authored
      Buildfarm members bowerbird and jacana have shown intermittent "could not
      bind IPv4 socket" failures in the BinInstallCheck stage since mid-December,
      shortly after commits 1caef31d and 9821492e changed the
      logic for selecting which port to use in temporary installations.  One
      plausible explanation is that we are randomly selecting ports that are
      already in use for some non-Postgres purpose.  Although the code tried
      to defend against already-in-use ports, it used pg_isready to probe
      the port which is quite unhelpful: if some non-Postgres server responds
      at the given address, pg_isready will generally say "no response",
      leading to exactly the wrong conclusion about whether the port is free.
      
      Instead, let's use a simple TCP connect() call to see if anything answers
      without making assumptions about what it is.  Note that this means there's
      no direct check for a conflicting Unix socket, but that should be okay
      because there should be no other Unix sockets in use in the temporary
      socket directory created for a test run.
      
      This is only a partial solution for the TCP case, since if the port number
      is in use for an outgoing connection rather than a listening socket, we'll
      fail to detect that.  We could try to bind() to the proposed port as a
      means of detecting that case, but that would introduce its own failure
      modes, since the system might consider the address to remain reserved for
      some period of time after we drop the bound socket.  Close study of the
      errors returned by bowerbird and jacana suggests that what we're seeing
      there may be conflicts with listening not outgoing sockets, so let's try
      this and see if it improves matters.  It's certainly better than what's
      there now, in any case.
      
      Michael Paquier, adjusted by me to work on non-Windows as well as Windows
      fab84c77
    • Andres Freund's avatar
      Fix documentation & config inconsistencies around 428b1d6b. · 8f91d87d
      Andres Freund authored
      Several issues:
      1) checkpoint_flush_after doc and code disagreed about the default
      2) new GUCs were missing from postgresql.conf.sample
      3) Outdated source-code comment about bgwriter_flush_after's default
      4) Sub-optimal categories assigned to new GUCs
      5) Docs suggested backend_flush_after is PGC_SIGHUP, but it's PGC_USERSET.
      6) Spell out int as integer in the docs, as done elsewhere
      
      Reported-By: Magnus Hagander, Fujii Masao
      Discussion: CAHGQGwETyTG5VYQQ5C_srwxWX7RXvFcD3dKROhvAWWhoSBdmZw@mail.gmail.com
      8f91d87d