1. 07 Sep, 2017 7 commits
    • Tom Lane's avatar
      Improve performance of get_actual_variable_range with recently-dead tuples. · 3ca930fc
      Tom Lane authored
      In commit fccebe42, we hacked get_actual_variable_range() to scan the
      index with SnapshotDirty, so that if there are many uncommitted tuples
      at the end of the index range, it wouldn't laboriously scan through all
      of them looking for a live value to return.  However, that didn't fix it
      for the case of many recently-dead tuples at the end of the index;
      SnapshotDirty recognizes those as committed dead and so we're back to
      the same problem.
      
      To improve the situation, invent a "SnapshotNonVacuumable" snapshot type
      and use that instead.  The reason this helps is that, if the snapshot
      rejects a given index entry, we know that the indexscan will mark that
      index entry as killed.  This means the next get_actual_variable_range()
      scan will proceed past that entry without visiting the heap, making the
      scan a lot faster.  We may end up accepting a recently-dead tuple as
      being the estimated extremal value, but that doesn't seem much worse than
      the compromise we made before to accept not-yet-committed extremal values.
      
      The cost of the scan is still proportional to the number of dead index
      entries at the end of the range, so in the interval after a mass delete
      but before VACUUM's cleaned up the mess, it's still possible for
      get_actual_variable_range() to take a noticeable amount of time, if you've
      got enough such dead entries.  But the constant factor is much much better
      than before, since all we need to do with each index entry is test its
      "killed" bit.
      
      We chose to back-patch commit fccebe42 at the time, but I'm hesitant to
      do so here, because this form of the problem seems to affect many fewer
      people.  Also, even when it happens, it's less bad than the case fixed
      by commit fccebe42 because we don't get the contention effects from
      expensive TransactionIdIsInProgress tests.
      
      Dmitriy Sarafannikov, reviewed by Andrey Borodin
      
      Discussion: https://postgr.es/m/05C72CF7-B5F6-4DB9-8A09-5AC897653113@yandex.ru
      3ca930fc
    • Tom Lane's avatar
      Improve documentation about behavior of multi-statement Query messages. · b9764994
      Tom Lane authored
      We've long done our best to sweep this topic under the rug, but in view
      of recent work it seems like it's time to explain it more precisely.
      Here's an initial cut at doing that.
      
      Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F6BE40D@G01JPEXMBYT05
      b9764994
    • Peter Eisentraut's avatar
      Reduce excessive dereferencing of function pointers · 1356f78e
      Peter Eisentraut authored
      It is equivalent in ANSI C to write (*funcptr) () and funcptr().  These
      two styles have been applied inconsistently.  After discussion, we'll
      use the more verbose style for plain function pointer variables, to make
      it clear that it's a variable, and the shorter style when the function
      pointer is in a struct (s.func() or s->func()), because then it's clear
      that it's not a plain function name, and otherwise the excessive
      punctuation makes some of those invocations hard to read.
      
      Discussion: https://www.postgresql.org/message-id/f52c16db-14ed-757d-4b48-7ef360b1631d@2ndquadrant.com
      1356f78e
    • Robert Haas's avatar
      Even if some partitions are foreign, allow tuple routing. · 9d71323d
      Robert Haas authored
      This doesn't allow routing tuple to the foreign partitions themselves,
      but it permits tuples to be routed to regular partitions despite the
      presence of foreign partitions in the same inheritance hierarchy.
      
      Etsuro Fujita, reviewed by Amit Langote and by me.
      
      Discussion: http://postgr.es/m/bc3db4c1-1693-3b8a-559f-33ad2b50b7ad@lab.ntt.co.jp
      9d71323d
    • Tom Lane's avatar
      Fix handling of savepoint commands within multi-statement Query strings. · 6eb52da3
      Tom Lane authored
      Issuing a savepoint-related command in a Query message that contains
      multiple SQL statements led to a FATAL exit with a complaint about
      "unexpected state STARTED".  This is a shortcoming of commit 4f896dac,
      which attempted to prevent such misbehaviors in multi-statement strings;
      its quick hack of marking the individual statements as "not top-level"
      does the wrong thing in this case, and isn't a very accurate description
      of the situation anyway.
      
      To fix, let's introduce into xact.c an explicit model of what happens for
      multi-statement Query strings.  This is an "implicit transaction block
      in progress" state, which for many purposes works like the normal
      TBLOCK_INPROGRESS state --- in particular, IsTransactionBlock returns true,
      causing the desired result that PreventTransactionChain will throw error.
      But in case of error abort it works like TBLOCK_STARTED, allowing the
      transaction to be cancelled without need for an explicit ROLLBACK command.
      
      Commit 4f896dac is reverted in toto, so that we go back to treating the
      individual statements as "top level".  We could have left it as-is, but
      this allows sharpening the error message for PreventTransactionChain
      calls inside functions.
      
      Except for getting a normal error instead of a FATAL exit for savepoint
      commands, this patch should result in no user-visible behavioral change
      (other than that one error message rewording).  There are some things
      we might want to do in the line of changing the appearance or wording of
      error and warning messages around this behavior, which would be much
      simpler to do now that it's an explicitly modeled state.  But I haven't
      done them here.
      
      Although this fixes a long-standing bug, no backpatch.  The consequences
      of the bug don't seem severe enough to justify the risk that this commit
      itself creates some new issue.
      
      Patch by me, but it owes something to previous investigation by
      Takayuki Tsunakawa, who also reported the bug in the first place.
      Also thanks to Michael Paquier for reviewing.
      
      Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F6BE40D@G01JPEXMBYT05
      6eb52da3
    • Tom Lane's avatar
      Further marginal hacking on generic atomic ops. · bfea9256
      Tom Lane authored
      In the generic atomic ops that rely on a loop around a CAS primitive,
      there's no need to force the initial read of the "old" value to be atomic.
      In the typically-rare case that we get a torn value, that simply means
      that the first CAS attempt will fail; but it will update "old" to the
      atomically-read value, so the next attempt has a chance of succeeding.
      It was already being done that way in pg_atomic_exchange_u64_impl(),
      but let's duplicate the approach in the rest.
      
      (Given the current coding of the pg_atomic_read functions, this change
      is a no-op anyway on popular platforms; it only makes a difference where
      pg_atomic_read_u64_impl() is implemented as a CAS.)
      
      In passing, also remove unnecessary take-a-pointer-and-dereference-it
      coding in the pg_atomic_read functions.  That seems to have been based
      on a misunderstanding of what the C standard requires.  What actually
      matters is that the pointer be declared as pointing to volatile, which
      it is.
      
      I don't believe this will change the assembly code at all on x86
      platforms (even ignoring the likelihood that these implementations
      get overridden by others); but it may help on less-mainstream CPUs.
      
      Discussion: https://postgr.es/m/13707.1504718238@sss.pgh.pa.us
      bfea9256
    • Simon Riggs's avatar
      Exclude special values in recovery_target_time · f06588a8
      Simon Riggs authored
      recovery_target_time accepts timestamp input, though
      does not allow use of special values, e.g. “today”.
      Report a useful error message for these cases.
      
      Reported-by: Piotr Stefaniak
      Author: Simon Riggs
      Discussion: https://postgr.es/m/CANP8+jJdKA+BkkYLWz9zAm16Y0s2ExBv0WfpAwXdTpPfWnA9Bg@mail.gmail.com
      f06588a8
  2. 06 Sep, 2017 9 commits
  3. 05 Sep, 2017 12 commits
  4. 04 Sep, 2017 4 commits
    • Tom Lane's avatar
      Be more careful about newline-chomping in pgbench. · 0b707d6e
      Tom Lane authored
      process_backslash_command would drop the last character of the input
      command on the assumption that it was a newline.  Given a non newline
      terminated input file, this could result in dropping the last character
      of the command.  Fix that by doing an actual test that we're removing
      a newline.
      
      While at it, allow for Windows newlines (\r\n), and suppress multiple
      newlines if any.  I do not think either of those cases really occur,
      since (a) we read script files in text mode and (b) the lexer stops
      when it hits a newline.  But it's cheap enough and it provides a
      stronger guarantee about what the result string looks like.
      
      This is just cosmetic, I think, since the possibly-overly-chomped
      line was only used for display not for further processing.  So
      it doesn't seem necessary to back-patch.
      
      Fabien Coelho, reviewed by Nikolay Shaplov, whacked around a bit by me
      
      Discussion: https://postgr.es/m/alpine.DEB.2.20.1704171422500.4025@lancre
      0b707d6e
    • Tom Lane's avatar
      Fix some subtle problems in pgbench transaction stats counting. · c23bb6ba
      Tom Lane authored
      With --latency-limit, transactions might get skipped even beyond the
      transaction count limit specified by -t, throwing off the expected
      number of transactions and thus the denominator for later stats.
      Be sure to stop skipping transactions once -t is reached.
      
      Also, include skipped transactions in the "cnt" fields; this requires
      discounting them again in a couple of places, but most places are
      better off with this definition.  In particular this is needed to
      get correct overall stats for the combination of -R/-L/-t options.
      Merge some more processing into processXactStats() to simplify this.
      
      In passing, add a check that --progress-timestamp is specified only
      when --progress is.
      
      We might consider back-patching this, but given that it only matters
      for a combination of options, and given the lack of field complaints,
      consensus seems to be not to bother.
      
      Fabien Coelho, reviewed by Nikolay Shaplov
      
      Discussion: https://postgr.es/m/alpine.DEB.2.20.1704171422500.4025@lancre
      c23bb6ba
    • Tom Lane's avatar
      Adjust pgbench to allow non-ASCII characters in variable names. · 9d36a386
      Tom Lane authored
      This puts it in sync with psql's notion of what is a valid variable name.
      Like psql, we document that "non-Latin letters" are allowed, but actually
      any non-ASCII character is accepted.
      
      Fabien Coelho
      
      Discussion: https://postgr.es/m/20170405.094548.1184280384967203518.t-ishii@sraoss.co.jp
      9d36a386
    • Alvaro Herrera's avatar
  5. 03 Sep, 2017 2 commits
  6. 02 Sep, 2017 1 commit
  7. 01 Sep, 2017 5 commits
    • Tom Lane's avatar
      Improve division of labor between execParallel.c and nodeGather[Merge].c. · 51daa7bd
      Tom Lane authored
      Move the responsibility for creating/destroying TupleQueueReaders into
      execParallel.c, to avoid duplicative coding in nodeGather.c and
      nodeGatherMerge.c.  Also, instead of having DestroyTupleQueueReader do
      shm_mq_detach, do it in the caller (which is now only ExecParallelFinish).
      This means execParallel.c does both the attaching and detaching of the
      tuple-queue-reader shm_mqs, which seems less weird than the previous
      arrangement.
      
      These changes also eliminate a vestigial memory leak (of the pei->tqueue
      array).  It's now demonstrable that rescans of Gather or GatherMerge don't
      leak memory.
      
      Discussion: https://postgr.es/m/8670.1504192177@sss.pgh.pa.us
      51daa7bd
    • Peter Eisentraut's avatar
      Add memory info to getrusage output · c039ba07
      Peter Eisentraut authored
      Add the maxrss field to the getrusage output (log_*_stats).  This was
      previously omitted because of portability concerns, but we feel this
      might not be a concern anymore.
      
      based on patch by Justin Pryzby <pryzby@telsasoft.com>
      c039ba07
    • Robert Haas's avatar
      Tighten up some code in RelationBuildPartitionDesc. · 0cb8b753
      Robert Haas authored
      This probably doesn't save anything meaningful in terms of
      performance, but making the code simpler is a good idea anyway.
      
      Code by Beena Emerson, extracted from a larger patch by Jeevan
      Ladhe, slightly adjusted by me.
      
      Discussion: http://postgr.es/m/CAOgcT0ONgwajdtkoq+AuYkdTPY9cLWWLjxt_k4SXue3eieAr+g@mail.gmail.com
      0cb8b753
    • Tom Lane's avatar
      Make [U]INT64CONST safe for use in #if conditions. · 9d6b160d
      Tom Lane authored
      Instead of using a cast to force the constant to be the right width,
      assume we can plaster on an L, UL, LL, or ULL suffix as appropriate.
      The old approach to this is very hoary, dating from before we were
      willing to require compilers to have working int64 types.
      
      This fix makes the PG_INT64_MIN, PG_INT64_MAX, and PG_UINT64_MAX
      constants safe to use in preprocessor conditions, where a cast
      doesn't work.  Other symbolic constants that might be defined using
      [U]INT64CONST are likewise safer than before.
      
      Also fix the SIZE_MAX macro to be similarly safe, if we are forced
      to provide a definition for that.  The test added in commit 2e70d6b5
      happens to do what we want even with the hack "(size_t) -1" definition,
      but we could easily get burnt on other tests in future.
      
      Back-patch to all supported branches, like the previous commits.
      
      Discussion: https://postgr.es/m/15883.1504278595@sss.pgh.pa.us
      9d6b160d
    • Peter Eisentraut's avatar
      doc: Remove mentions of server-side CRL and CA file names · a0572203
      Peter Eisentraut authored
      Commit a445cb92 removed the default file
      names for server-side CRL and CA files, but left them in the docs with a
      small note.  This removes the note and the previous default names to
      clarify, as well as changes mentions of the file names to make it
      clearer that they are configurable.
      
      Author: Daniel Gustafsson <daniel@yesql.se>
      Reviewed-by: default avatarMichael Paquier <michael.paquier@gmail.com>
      a0572203