aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Add a WAIT option to DROP_REPLICATION_SLOTAlvaro Herrera2017-09-01
| | | | | | | | | | | Commit 9915de6c1cb2 changed the default behavior of DROP_REPLICATION_SLOT so that it would wait until any session holding the slot active would release it, instead of raising an error. But users are already depending on the original behavior, so revert to it by default and add a WAIT option to invoke the new behavior. Per complaint from Simone Gotti, in Discussion: https://postgr.es/m/CAEvsy6Wgdf90O6pUvg2wSVXL2omH5OPC-38OD4Zzgk-FXavj3Q@mail.gmail.com
* Avoid memory leaks when a GatherMerge node is rescanned.Tom Lane2017-08-31
| | | | | | | | | | | | | | | | | | | | | | | | Rescanning a GatherMerge led to leaking some memory in the executor's query-lifespan context, because most of the node's working data structures were simply abandoned and rebuilt from scratch. In practice, this might never amount to much, given the cost of relaunching worker processes --- but it's still pretty messy, so let's fix it. We can rearrange things so that the tuple arrays are simply cleared and reused, and we don't need to rebuild the TupleTableSlots either, just clear them. One small complication is that because we might get a different number of workers on each iteration, we can't keep the old convention that the leader's gm_slots[] entry is the last one; the leader might clobber a TupleTableSlot that we need for a worker in a future iteration. Hence, adjust the logic so that the leader has slot 0 always, while the active workers have slots 1..n. Back-patch to v10 to keep all the existing versions of nodeGatherMerge.c in sync --- because of the renumbering of the slots, there would otherwise be a very large risk that any future backpatches in this module would introduce bugs. Discussion: https://postgr.es/m/8670.1504192177@sss.pgh.pa.us
* Clean up shm_mq cleanup.Tom Lane2017-08-31
| | | | | | | | | | | | | | | | | | | | | | | | | The logic around shm_mq_detach was a few bricks shy of a load, because (contrary to the comments for shm_mq_attach) all it did was update the shared shm_mq state. That left us leaking a bit of process-local memory, but much worse, the on_dsm_detach callback for shm_mq_detach was still armed. That means that whenever we ultimately detach from the DSM segment, we'd run shm_mq_detach again for already-detached, possibly long-dead queues. This accidentally fails to fail today, because we only ever re-use a shm_mq's memory for another shm_mq, and multiple detach attempts on the last such shm_mq are fairly harmless. But it's gonna bite us someday, so let's clean it up. To do that, change shm_mq_detach's API so it takes a shm_mq_handle not the underlying shm_mq. This makes the callers simpler in most cases anyway. Also fix a few places in parallel.c that were just pfree'ing the handle structs rather than doing proper cleanup. Back-patch to v10 because of the risk that the revenant shm_mq_detach callbacks would cause a live bug sometime. Since this is an API change, it's too late to do it in 9.6. (We could make a variant patch that preserves API, but I'm not excited enough to do that.) Discussion: https://postgr.es/m/8670.1504192177@sss.pgh.pa.us
* Improve code coverage of select_parallel test.Tom Lane2017-08-31
| | | | | Make sure that rescans of parallel indexscans are tested. Per code coverage report.
* Code review for nodeGatherMerge.c.Tom Lane2017-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Comment the fields of GatherMergeState, and organize them a bit more sensibly. Comment GMReaderTupleBuffer more usefully too. Improve assorted other comments that were obsolete or just not very good English. Get rid of the use of a GMReaderTupleBuffer for the leader process; that was confusing, since only the "done" field was used, and that in a way redundant with need_to_scan_locally. In gather_merge_init, avoid calling load_tuple_array for already-known-exhausted workers. I'm not sure if there's a live bug there, but the case is unlikely to be well tested due to timing considerations. Remove some useless code, such as duplicating the tts_isempty test done by TupIsNull. Remove useless initialization of ps.qual, replacing that with an assertion that we have no qual to check. (If we did, the code would fail to check it.) Avoid applying heap_copytuple to a null tuple. While that fails to crash, it's confusing and it makes the code less legible not more so IMO. Propagate a couple of these changes into nodeGather.c, as well. Back-patch to v10, partly because of the possibility that the gather_merge_init change is fixing a live bug, but mostly to keep the branches in sync to ease future bug fixes.
* Separate reinitialization of shared parallel-scan state from ExecReScan.Tom Lane2017-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the parallel executor logic did reinitialization of shared state within the ExecReScan code for parallel-aware scan nodes. This is problematic, because it means that the ExecReScan call has to occur synchronously (ie, during the parent Gather node's ReScan call). That is swimming very much against the tide so far as the ExecReScan machinery is concerned; the fact that it works at all today depends on a lot of fragile assumptions, such as that no plan node between Gather and a parallel-aware scan node is parameterized. Another objection is that because ExecReScan might be called in workers as well as the leader, hacky extra tests are needed in some places to prevent unwanted shared-state resets. Hence, let's separate this code into two functions, a ReInitializeDSM call and the ReScan call proper. ReInitializeDSM is called only in the leader and is guaranteed to run before we start new workers. ReScan is returned to its traditional function of resetting only local state, which means that ExecReScan's usual habits of delaying or eliminating child rescan calls are safe again. As with the preceding commit 7df2c1f8d, it doesn't seem to be necessary to make these changes in 9.6, which is a good thing because the FDW and CustomScan APIs are impacted. Discussion: https://postgr.es/m/CAA4eK1JkByysFJNh9M349u_nNjqETuEnY_y1VUc_kJiU0bxtaQ@mail.gmail.com
* Restore test case from a2b70c89ca1a5fcf6181d3c777d82e7b83d2de1b.Tom Lane2017-08-30
| | | | | | | | Revert the reversion commits a20aac890 and 9b644745c. In the wake of commit 7df2c1f8d, we should get stable buildfarm results from this test; if not, I'd like to know sooner not later. Discussion: https://postgr.es/m/CAA4eK1JkByysFJNh9M349u_nNjqETuEnY_y1VUc_kJiU0bxtaQ@mail.gmail.com
* Force rescanning of parallel-aware scan nodes below a Gather[Merge].Tom Lane2017-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The ExecReScan machinery contains various optimizations for postponing or skipping rescans of plan subtrees; for example a HashAgg node may conclude that it can re-use the table it built before, instead of re-reading its input subtree. But that is wrong if the input contains a parallel-aware table scan node, since the portion of the table scanned by the leader process is likely to vary from one rescan to the next. This explains the timing-dependent buildfarm failures we saw after commit a2b70c89c. The established mechanism for showing that a plan node's output is potentially variable is to mark it as depending on some runtime Param. Hence, to fix this, invent a dummy Param (one that has a PARAM_EXEC parameter number, but carries no actual value) associated with each Gather or GatherMerge node, mark parallel-aware nodes below that node as dependent on that Param, and arrange for ExecReScanGather[Merge] to flag that Param as changed whenever the Gather[Merge] node is rescanned. This solution breaks an undocumented assumption made by the parallel executor logic, namely that all rescans of nodes below a Gather[Merge] will happen synchronously during the ReScan of the top node itself. But that's fundamentally contrary to the design of the ExecReScan code, and so was doomed to fail someday anyway (even if you want to argue that the bug being fixed here wasn't a failure of that assumption). A follow-on patch will address that issue. In the meantime, the worst that's expected to happen is that given very bad timing luck, the leader might have to do all the work during a rescan, because workers think they have nothing to do, if they are able to start up before the eventual ReScan of the leader's parallel-aware table scan node has reset the shared scan state. Although this problem exists in 9.6, there does not seem to be any way for it to manifest there. Without GatherMerge, it seems that a plan tree that has a rescan-short-circuiting node below Gather will always also have one above it that will short-circuit in the same cases, preventing the Gather from being rescanned. Hence we won't take the risk of back-patching this change into 9.6. But v10 needs it. Discussion: https://postgr.es/m/CAA4eK1JkByysFJNh9M349u_nNjqETuEnY_y1VUc_kJiU0bxtaQ@mail.gmail.com
* Teach libpq to detect integer overflow in the row count of a PGresult.Tom Lane2017-08-29
| | | | | | | | | | | | | | | | | | | | | | | | | | Adding more than 1 billion rows to a PGresult would overflow its ntups and tupArrSize fields, leading to client crashes. It'd be desirable to use wider fields on 64-bit machines, but because all of libpq's external APIs use plain "int" for row counters, that's going to be hard to accomplish without an ABI break. Given the lack of complaints so far, and the general pain that would be involved in using such huge PGresults, let's settle for just preventing the overflow and reporting a useful error message if it does happen. Also, for a couple more lines of code we can increase the threshold of trouble from INT_MAX/2 to INT_MAX rows. To do that, refactor pqAddTuple() to allow returning an error message that replaces the default assumption that it failed because of out-of-memory. Along the way, fix PQsetvalue() so that it reports all failures via pqInternalNotice(). It already did so in the case of bad field number, but neglected to report anything for other error causes. Because of the potential for crashes, this seems like a back-patchable bug fix, despite the lack of field reports. Michael Paquier, per a complaint from Igor Korot. Discussion: https://postgr.es/m/CA+FnnTxyLWyjY1goewmJNxC==HQCCF4fKkoCTa9qR36oRAHDPw@mail.gmail.com
* Stamp 10beta4.REL_10_BETA4Tom Lane2017-08-28
|
* Translation updatesPeter Eisentraut2017-08-28
| | | | | Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 31ad7831c3018858b662ed1d26a6c3bfe92b4e1f
* Fix over-aggressive sanity check in misc_sanity.sql.Tom Lane2017-08-28
| | | | | | | | | | | | | | Fix thinko in commit 8be8510cf: it's okay to have dbid == 0 in normal (non-pin) entries in pg_shdepend, because global objects such as databases are entered that way. The test would pass so long as it was run in a cluster containing no databases/tablespaces owned by, or granted to, roles other than the bootstrap superuser. That's the expected situation for "make check", but for "make installcheck", not so much. Reported by Ryan Murphy. Discussion: https://postgr.es/m/CAHeEsBc6EQe0mxGBKDXAwJbntgfvoAd5MQC-5362SmC3Tng_6g@mail.gmail.com
* pg_test_timing: Some NLS fixesPeter Eisentraut2017-08-26
| | | | | | | | | The string "% of total" was marked by xgettext to be a c-format, but it is actually not, so mark up the source to prevent that. Compute the column widths of the final display dynamically based on the translated strings, so that translations don't mess up the display accidentally.
* pg_upgrade: Remove more dead codePeter Eisentraut2017-08-25
| | | | | | related to 6ce6a61840cc90172ad3da7bf303656132fa5fab Reported-by: Christoph Berg <myon@debian.org>
* Message translatability fixesPeter Eisentraut2017-08-25
|
* Fix harmless thinko in dsa.c.Andres Freund2017-08-24
| | | | | | | | | | | | Commit 16be2fd100199bdf284becfcee02c5eb20d8a11d added DSA_ALLOC_HUGE, DSA_ALLOC_ZERO and DSA_ALLOC_NO_OOM which have the same numerical values and meanings as the similarly named MCXT_... macros. In one place we accidentally used MCXT_ALLOC_NO_OOM when DSA_ALLOC_NO_OOM is wanted, so tidy that up. Author: Thomas Munro Discussion: http://postgr.es/m/CAEepm=2AimHxVkkxnMfQvbZMkXy0uKbVa0-D38c5-qwrCm4CMQ@mail.gmail.com Backpatch: 10, where dsa was introduced.
* psql: Fix \gx when FETCH_COUNT is usedStephen Frost2017-08-24
| | | | | | | | Set expanded output when requested through \gx in ExecQueryUsingCursor() (used when FETCH_COUNT is set). Discussion: https://www.postgresql.org/message-id/CB7A53AA-5645-4BDD-AB07-4D22CD9D8FF1%40gmx.net Author: Tobias Bussmann
* pg_upgrade: Remove dead codePeter Eisentraut2017-08-24
| | | | | | Remove code meant for upgrading to a particular version of PostgreSQL 9.0. Since pg_upgrade only supports upgrading to the current major version, this code is no longer useful.
* Increase SCRAM salt lengthPeter Eisentraut2017-08-24
| | | | | | | | The original value 12 was set based on RFC 5802 for SCRAM-SHA-1, but RFC 7677 for SCRAM-SHA-256 uses 16, so use that. (This does not affect the validity of already stored verifiers.) Discussion: https://www.postgresql.org/message-id/flat/12cc9297-7e05-932f-d863-765e5626ead4%402ndquadrant.com
* Update code comment for temporary replication slotsPeter Eisentraut2017-08-23
| | | | Reported-by: Alvaro Herrera <alvherre@2ndquadrant.com>
* Fix outdated commentPeter Eisentraut2017-08-23
| | | | Author: Thomas Munro <thomas.munro@enterprisedb.com>
* Tweak some SCRAM error messages and code commentsPeter Eisentraut2017-08-23
| | | | | | Clarify/correct some error messages, fix up some code comments that confused SASL and SCRAM, and other minor fixes. No changes in functionality.
* Fix translation markerPeter Eisentraut2017-08-23
| | | | | This was erroneously removed in 55a70a023c3daefca9bbd68bfbe6862af10ab479.
* pg_upgrade: Message translatability and style fixesPeter Eisentraut2017-08-22
|
* Backpatch introduction of TupleDescAttr(tupdesc, i).Andres Freund2017-08-22
| | | | | | | | | | | | | 2cd70845240 / c6293249d change the way individual attributes in a TupleDesc are stored / accessed. To reduce the effort of making extensions compatible with postgresql 11, and to ease future backpatching, backpatch introduction of TupleDescAttr() to all releases. Do not backpatch change in storage, as that'd be a breaking change for existing and working extensions. Author: Andres Freund Discussion: https://postgr.es/m/20170820181723.tdswdinzptbcwhrr@alap3.anarazel.de Backpatch: 9.2-
* Don't install ICU collation keyword variantsPeter Eisentraut2017-08-21
| | | | | | | Users can still create them themselves. Instead, document Unicode TR 35 collation options for ICU, so users can create all this themselves. Reviewed-by: Peter Geoghegan <pg@bowt.ie>
* Expand set of predefined ICU localesPeter Eisentraut2017-08-21
| | | | | | | | | Install language+region combinations even if they are not distinct from the language's base locale. This gives better long-term stability of the set of predefined locales and makes the predefined locales less implementation-dependent and more practical for users. Reviewed-by: Peter Geoghegan <pg@bowt.ie>
* Inject $(ICU_LIBS) regardless of platform.Noah Misch2017-08-20
| | | | | | It appeared in a conditional that excludes AIX, Cygwin and MinGW. Give ICU support a chance to work on those platforms. Back-patch to v10, where ICU support was introduced.
* Fix possible core dump in parallel restore when using a TOC list.Tom Lane2017-08-19
| | | | | | | | | | | | | | | | | | | | | | | | | Commit 3eb9a5e7c unintentionally introduced an ordering dependency into restore_toc_entries_prefork(). The existing coding of reduce_dependencies() contains a check to skip moving a TOC entry to the ready_list if it wasn't initially in the pending_list. This used to suffice to prevent reduce_dependencies() from trying to move anything into the ready_list during restore_toc_entries_prefork(), because the pending_list stayed empty throughout that phase; but it no longer does. The problem doesn't manifest unless the TOC has been reordered by SortTocFromFile, which is how I missed it in testing. To fix, just add a test for ready_list == NULL, converting the call with NULL from a poor man's sanity check into an explicit command not to touch TOC items' list membership. Clarify some of the comments around this; in particular, note the primary purpose of the check for pending_list membership, which is to ensure that we can't try to restore the same item twice, in case a TOC list forces it to be restored before its dependency count goes to zero. Per report from Fabrízio de Royes Mello. Back-patch to 9.3, like the previous commit. Discussion: https://postgr.es/m/CAFcNs+pjuv0JL_x4+=71TPUPjdLHOXA4YfT32myj_OrrZb4ohA@mail.gmail.com
* Fix creation of ICU comments for keyword variantsPeter Eisentraut2017-08-18
| | | | | It would create the comment referring to the keyword-less parent locale. This was broken in ddb5fdc068635d003a0d1c303cb109d1cb3ebeb1.
* Fix interaction of triggers, partitioning, and EXPLAIN ANALYZE.Robert Haas2017-08-18
| | | | | | | | | | | Add a new EState member es_leaf_result_relations, so that the trigger code knows about ResultRelInfos created by tuple routing. Also make sure ExplainPrintTriggers knows about partition-related ResultRelInfos. Etsuro Fujita, reviewed by Amit Langote Discussion: http://postgr.es/m/57163e18-8e56-da83-337a-22f2c0008051@lab.ntt.co.jp
* Temporarily revert test case from a2b70c89ca1a5fcf6181d3c777d82e7b83d2de1b.Tom Lane2017-08-17
| | | | | | | | That code patch was good as far as it went, but the associated test case has exposed fundamental brain damage in the parallel scan mechanism, which is going to take nontrivial work to correct. In the interests of getting the buildfarm back to green so that unrelated work can proceed, let's temporarily remove the test case.
* Don't lock tables in RelationGetPartitionDispatchInfo.Robert Haas2017-08-17
| | | | | | | | | | | | Instead, lock them in the caller using find_all_inheritors so that they get locked in the standard order, minimizing deadlock risks. Also in RelationGetPartitionDispatchInfo, avoid opening tables which are not partitioned; there's no need. Amit Langote, reviewed by Ashutosh Bapat and Amit Khandekar Discussion: http://postgr.es/m/91b36fa1-c197-b72f-ca6e-56c593bae68c@lab.ntt.co.jp
* Fix ExecReScanGatherMerge.Tom Lane2017-08-17
| | | | | | | | | Not surprisingly, since it'd never ever been tested, ExecReScanGatherMerge didn't work. Fix it, and add a regression test case to exercise it. Amit Kapila Discussion: https://postgr.es/m/CAA4eK1JkByysFJNh9M349u_nNjqETuEnY_y1VUc_kJiU0bxtaQ@mail.gmail.com
* Further tweaks to compiler flags for PL/Perl on Windows.Tom Lane2017-08-17
| | | | | | | | | | | | | | | | It now emerges that we can only rely on Perl to tell us we must use -D_USE_32BIT_TIME_T if it's Perl 5.13.4 or later. For older versions, revert to our previous practice of assuming we need that symbol in all 32-bit Windows builds. This is not ideal, but inquiring into which compiler version Perl was built with seems far too fragile. In any case, we had not previously had complaints about these old Perl versions, so let's assume this is Good Enough. (It's still better than the situation ante commit 5a5c2feca, in that at least the effects are confined to PL/Perl rather than the whole PG build.) Back-patch to all supported versions, like 5a5c2feca and predecessors. Discussion: https://postgr.es/m/CANFyU97OVQ3+Mzfmt3MhuUm5NwPU=-FtbNH5Eb7nZL9ua8=rcA@mail.gmail.com
* pg_dump: Support using synchronized snapshots on standbysPeter Eisentraut2017-08-16
| | | | | | | | This became possible by commit 6c2003f8a1bbc7c192a2e83ec51581c018aa162f. This just makes pg_dump aware of it and updates the documentation. Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
* Allow continuation lines in ecpg cppline parsing.Michael Meskes2017-08-16
|
* Initialize replication_slot_catalog_xmin in procarrayPeter Eisentraut2017-08-15
| | | | | | | | | | | Although not confirmed and probably rare, if the newly allocated memory is not already zero, this could possibly have caused some problems. Also reorder the initializations slightly so they match the order of the struct definition. Author: Wong, Yi Wen <yiwong@amazon.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
* Include foreign tables in information_schema.table_privilegesPeter Eisentraut2017-08-15
| | | | | | | | This appears to have been an omission in the original commit 0d692a0dc9f. All related information_schema views already include foreign tables. Reported-by: Nicolas Thauvin <nicolas.thauvin@dalibo.com>
* psql: Add tab completion for \pset pagerPeter Eisentraut2017-08-15
| | | | Author: Pavel Stehule <pavel.stehule@gmail.com>
* Simplify autovacuum work-item implementationAlvaro Herrera2017-08-15
| | | | | | | | | | | | | The initial implementation of autovacuum work-items used a dynamic shared memory area (DSA). However, it's argued that dynamic shared memory is not portable enough, so we cannot rely on it being supported everywhere; at the same time, autovacuum work-items are now a critical part of the server, so it's not acceptable that they don't work in the cases where dynamic shared memory is disabled. Therefore, let's fall back to a simpler implementation of work-items that just uses autovacuum's main shared memory segment for storage. Discussion: https://postgr.es/m/CA+TgmobQVbz4K_+RSmiM9HeRKpy3vS5xnbkL95gSEnWijzprKQ@mail.gmail.com
* Fix logical replication protocol comparison logicPeter Eisentraut2017-08-15
| | | | | | | Since we currently only have one protocol, this doesn't make much of a difference other than the error message. Author: Yugo Nagata <nagata@sraoss.co.jp>
* Simplify some code in logical replication launcherPeter Eisentraut2017-08-15
| | | | | | Avoid unnecessary locking calls when a subscription is disabled. Author: Yugo Nagata <nagata@sraoss.co.jp>
* Fix error handling path in autovacuum launcherAlvaro Herrera2017-08-15
| | | | | | | | | | | The original code (since 00e6a16d01) was assuming aborting the transaction in autovacuum launcher was sufficient to release all resources, but in reality the launcher runs quite a lot of code out of any transactions. Re-introduce individual cleanup calls to make abort more robust. Reported-by: Robert Haas Discussion: https://postgr.es/m/CA+TgmobQVbz4K_+RSmiM9HeRKpy3vS5xnbkL95gSEnWijzprKQ@mail.gmail.com
* Distinguish wait-for-connection from wait-for-write-ready on Windows.Tom Lane2017-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The API for WaitLatch and friends followed the Unix convention in which waiting for a socket connection to complete is identical to waiting for the socket to accept a write. While Windows provides a select(2) emulation that agrees with that, the native WaitForMultipleObjects API treats them as quite different --- and for some bizarre reason, it will report a not-yet-connected socket as write-ready. libpq itself has so far escaped dealing with this because it waits with select(), but in libpqwalreceiver.c we want to wait using WaitLatchOrSocket. The semantics mismatch resulted in replication connection failures on Windows, but only for remote connections (apparently, localhost connections complete immediately, or at least too fast for anyone to have noticed the problem in single-machine testing). To fix, introduce an additional WL_SOCKET_CONNECTED wait flag for WaitLatchOrSocket, which is identical to WL_SOCKET_WRITEABLE on non-Windows, but results in waiting for FD_CONNECT events on Windows. Ideally, we would also distinguish the two conditions in the API for PQconnectPoll(), but changing that API at this point seems infeasible. Instead, cheat by checking for PQstatus() == CONNECTION_STARTED to determine that we're still waiting for the connection to complete. (This is a cheat mainly because CONNECTION_STARTED is documented as an internal state rather than something callers should rely on. Perhaps we ought to change the documentation ... but this patch doesn't.) Per reports from Jobin Augustine and Igor Neyman. Back-patch to v10 where commit 1e8a85009 exposed this longstanding shortcoming. Andres Freund, minor fix and some code review/beautification by me Discussion: https://postgr.es/m/CAHBggj8g2T+ZDcACZ2FmzX9CTxkWjKBsHd6NkYB4i9Ojf6K1Fw@mail.gmail.com
* Avoid unnecessary single-child Append nodes.Robert Haas2017-08-15
| | | | | | | | | | | | | Before commit d3cc37f1d801a6b5cad9bf179274a8, an inheritance parent whose only children were temp tables of other sessions would end up as a simple scan of the parent; but with that commit, we end up with an Append node, per a report from Ashutosh Bapat. Tweak the logic so that we go back to the old way, and update the function header comment for partitioning while we're at it. Ashutosh Bapat, reviewed by Amit Langote and adjusted by me. Discussion: http://postgr.es/m/CAFjFpReWJr1yTkHU=OqiMBmcYCMoSW3VPR39RBuQ_ovwDFBT5Q@mail.gmail.com
* Add missing call to ExecReScanGatherMerge.Robert Haas2017-08-15
| | | | | | Amit Kapila Discussion: http://postgr.es/m/CAA4eK1KeQWZOoDmDmGMwuqzPW9JhRS+ditQVFdAfGjNmMZzqMQ@mail.gmail.com
* Expand coverage of parallel gather merge a bit.Andres Freund2017-08-14
| | | | | | | | | | | Previously paths reaching heap_compare_slots weren't covered. Author: Rushabh Lathia Reviewed-By: Andres Freund Discussion: https://postgr.es/m/CAGPqQf3C+3PBujb+7m=ceWeii4-vBY=XS99LjzrpkpefvzJbFg@mail.gmail.com https://postgr.es/m/27200.1502482851@sss.pgh.pa.us Backpatch: 10, where gather merge was introduced
* Final pgindent + perltidy run for v10.Tom Lane2017-08-14
|
* Handle elog(FATAL) during ROLLBACK more robustly.Tom Lane2017-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stress testing by Andreas Seltenreich disclosed longstanding problems that occur if a FATAL exit (e.g. due to receipt of SIGTERM) occurs while we are trying to execute a ROLLBACK of an already-failed transaction. In such a case, xact.c is in TBLOCK_ABORT state, so that AbortOutOfAnyTransaction would skip AbortTransaction and go straight to CleanupTransaction. This led to an assert failure in an assert-enabled build (due to the ROLLBACK's portal still having a cleanup hook) or without assertions, to a FATAL exit complaining about "cannot drop active portal". The latter's not disastrous, perhaps, but it's messy enough to want to improve it. We don't really want to run all of AbortTransaction in this code path. The minimum required to clean up the open portal safely is to do AtAbort_Memory and AtAbort_Portals. It seems like a good idea to do AtAbort_Memory unconditionally, to be entirely sure that we are starting with a safe CurrentMemoryContext. That means that if the main loop in AbortOutOfAnyTransaction does nothing, we need an extra step at the bottom to restore CurrentMemoryContext = TopMemoryContext, which I chose to do by invoking AtCleanup_Memory. This'll result in calling AtCleanup_Memory twice in many of the paths through this function, but that seems harmless and reasonably inexpensive. The original motivation for the assertion in AtCleanup_Portals was that we wanted to be sure that any user-defined code executed as a consequence of the cleanup hook runs during AbortTransaction not CleanupTransaction. That still seems like a valid concern, and now that we've seen one case of the assertion firing --- which means that exactly that would have happened in a production build --- let's replace the Assert with a runtime check. If we see the cleanup hook still set, we'll emit a WARNING and just drop the hook unexecuted. This has been like this a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/877ey7bmun.fsf@ansel.ydns.eu
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-05 02:48:42 +0000