127 Commits (78bc85dd4455c302c345c550e0628a1522df108d)

Author	SHA1	Message	Date
Masahiko Sawada	fc0fb77c55	Fix re-distributing previously distributed invalidation messages during logical decoding. ... Commit 4909b38af0 introduced logic to distribute invalidation messages from catalog-modifying transactions to all concurrent in-progress transactions. However, since each transaction distributes not only its original invalidation messages but also previously distributed messages to other transactions, this leads to an exponential increase in allocation request size for invalidation messages, ultimately causing memory allocation failure. This commit fixes this issue by tracking distributed invalidation messages separately per decoded transaction and not redistributing these messages to other in-progress transactions. The maximum size of distributed invalidation messages that one transaction can store is limited to MAX_DISTR_INVAL_MSG_PER_TXN (8MB). Once the size of the distributed invalidation messages exceeds this threshold, we invalidate all caches in locations where distributed invalidation messages need to be executed. Back-patch to all supported versions where we introduced the fix by commit 4909b38af0. Note that this commit adds two new fields to ReorderBufferTXN to store the distributed transactions. This change breaks ABI compatibility in back branches, affecting third-party extensions that depend on the size of the ReorderBufferTXN struct, though this scenario seems unlikely. Additionally, it adds a new flag to the txn_flags field of ReorderBufferTXN to indicate distributed invalidation message overflow. This should not affect existing implementations, as it is unlikely that third-party extensions use unused bits in the txn_flags field. Bug: #18938 #18942 Author: vignesh C <vignesh21@gmail.com> Reported-by: Duncan Sands <duncan.sands@deepbluecap.com> Reported-by: John Hutchins <john.hutchins@wicourts.gov> Reported-by: Laurence Parry <greenreaper@hotmail.com> Reported-by: Max Madden <maxmmadden@gmail.com> Reported-by: Braulio Fdo Gonzalez <brauliofg@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://postgr.es/m/680bdaf6-f7d1-4536-b580-05c2760c67c6@deepbluecap.com Discussion: https://postgr.es/m/18942-0ab1e5ae156613ad@postgresql.org Discussion: https://postgr.es/m/18938-57c9a1c463b68ce0@postgresql.org Discussion: https://postgr.es/m/CAD1FGCT2sYrP_70RTuo56QTizyc+J3wJdtn2gtO3VttQFpdMZg@mail.gmail.com Discussion: https://postgr.es/m/CANO2=B=2BT1hSYCE=nuuTnVTnjidMg0+-FfnRnqM6kd23qoygg@mail.gmail.com Backpatch-through: 13	9 months ago
Amit Kapila	f6429bd7db	Fix xmin advancement during fast_forward decoding. ... During logical decoding, we advance catalog_xmin of logical too early in fast_forward mode, resulting in required catalog data being removed by vacuum. This mode is normally used to advance the slot without processing the changes, but we still can't let the slot's xmin to advance to an incorrect value. Commit f49a80c481 fixed a similar issue where the logical slot's catalog_xmin was getting advanced prematurely during non-fast-forward mode. During xl_running_xacts processing, instead of directly advancing the slot's xmin to the oldest running xid in the record, it allowed the xmin to be held back for snapshots that can be used for not-yet-replayed transactions, as those might consider older txns as running too. However, it missed the fact that the same problem can happen during fast_forward mode decoding, as we won't build a base snapshot in that mode, and the future call to get_changes from the same slot can miss seeing the required catalog changes leading to incorrect reslts. This commit allows building the base snapshot even in fast_forward mode to prevent the early advancement of xmin. Reported-by: Amit Kapila <amit.kapila16@gmail.com> Author: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: shveta malik <shveta.malik@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/CAA4eK1LqWncUOqKijiafe+Ypt1gQAQRjctKLMY953J79xDBgAg@mail.gmail.com Discussion: https://postgr.es/m/OS0PR01MB57163087F86621D44D9A72BF94BB2@OS0PR01MB5716.jpnprd01.prod.outlook.com	11 months ago
Amit Kapila	90bc4523fd	Fix typo in test file name added in commit 4909b38af0. ... Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/CANhcyEXsObdjkjxEnq10aJumDpa5J6aiPzgTh_w4KCWRYHLw6Q@mail.gmail.com	11 months ago
Amit Kapila	9f21be08e8	Fix data loss in logical replication. ... Data loss can happen when the DDLs like ALTER PUBLICATION ... ADD TABLE ... or ALTER TYPE ... that don't take a strong lock on table happens concurrently to DMLs on the tables involved in the DDL. This happens because logical decoding doesn't distribute invalidations to concurrent transactions and those transactions use stale cache data to decode the changes. The problem becomes bigger because we keep using the stale cache even after those in-progress transactions are finished and skip the changes required to be sent to the client. This commit fixes the issue by distributing invalidation messages from catalog-modifying transactions to all concurrent in-progress transactions. This allows the necessary rebuild of the catalog cache when decoding new changes after concurrent DDL. We observed performance regression primarily during frequent execution of *publication DDL* statements that modify the published tables. The regression is minor or nearly nonexistent for DDLs that do not affect the published tables or occur infrequently, making this a worthwhile cost to resolve a longstanding data loss issue. An alternative approach considered was to take a strong lock on each affected table during publication modification. However, this would only address issues related to publication DDLs (but not the ALTER TYPE ...) and require locking every relation in the database for publications created as FOR ALL TABLES, which is impractical. The bug exists in all supported branches, but we are backpatching till 14. The fix for 13 requires somewhat bigger changes than this fix, so the fix for that branch is still under discussion. Reported-by: hubert depesz lubaczewski <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> Backpatch-through: 14 Discussion: https://postgr.es/m/de52b282-1166-1180-45a2-8d8917ca74c6@enterprisedb.com Discussion: https://postgr.es/m/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=kLQ@mail.gmail.com	11 months ago
Amit Kapila	5ce0dcc99a	Stabilize the test added by commit 022564f60c. ... The test was unstable in branches 14 and 15 as we were relying on the number of changes in the table having a toast column to start streaming. On branches >= 16, we have a GUC debug_logical_replication_streaming which can stream each change, so the test was stable in those branches. Change the test to use PREPARE TRANSACTION as that should make the result consistent and test the code changed in 022564f60c. Reported-by: Daniel Gustafsson as per buildfarm Author: Hou Zhijie, Amit Kapila Backpatch-through: 14 Discussion: https://postgr.es/m/8C2F86AA-981E-4803-B14D-E264C0255330@yesql.se	1 year ago
Amit Kapila	8175a7d11f	Fix fetching default toast value during decoding of in-progress transactions. ... During logical decoding of in-progress transactions, we perform the toast table scan while fetching the default toast value for an attribute. We forgot to initialize the flag during this scan to indicate that the system table scan is in progress. We need this flag to ensure that during logical decoding we never directly access the tableam or heap APIs because we check for concurrent aborts only in systable_* APIs. Reported-by: Alexander Lakhin Author: Takeshi Ideriha, Hou Zhijie Reviewed-by: Amit Kapila, Hou Zhijie Backpatch-through: 14 Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org	1 year ago
Masahiko Sawada	aee8c2b954	Fix possibility of logical decoding partial transaction changes. ... When creating and initializing a logical slot, the restart_lsn is set to the latest WAL insertion point (or the latest replay point on standbys). Subsequently, WAL records are decoded from that point to find the start point for extracting changes in the DecodingContextFindStartpoint() function. Since the initial restart_lsn could be in the middle of a transaction, the start point must be a consistent point where we won't see the data for partial transactions. Previously, when not building a full snapshot, serialized snapshots were restored, and the SnapBuild jumps to the consistent state even while finding the start point. Consequently, the slot's restart_lsn and confirmed_flush could be set to the middle of a transaction. This could lead to various unexpected consequences. Specifically, there were reports of logical decoding decoding partial transactions, and assertion failures occurred because only subtransactions were decoded without decoding their top-level transaction until decoding the commit record. To resolve this issue, the changes prevent restoring the serialized snapshot and jumping to the consistent state while finding the start point. On v17 and HEAD, a flag indicating whether snapshot restores should be skipped has been added to the SnapBuild struct, and SNAPBUILD_VERSION has been bumpded. On backbranches, the flag is stored in the LogicalDecodingContext instead, preserving on-disk compatibility. Backpatch to all supported versions. Reported-by: Drew Callahan Reviewed-by: Amit Kapila, Hayato Kuroda Discussion: https://postgr.es/m/2444AA15-D21B-4CCE-8052-52C7C2DAFE5C%40amazon.com Backpatch-through: 12	2 years ago
Amit Kapila	b793a416bf	Fix catalog lookup due to wrong snapshot for subtransactions during decoding. ... In commit 272248a0c, we fixed the catalog lookup due to the wrong snapshot for transactions and subtransactions during decoding. We failed to consider the case where top-level xact is already marked as containing catalog change but its subtransaction is not yet marked as containing catalog change even though it contained such a change. This can happen when during decoding, none of the WAL records from the subtransaction was decoded and top-level xact contains a DDL. We fix it by marking the transaction and all its subtransactions as containing catalog changes if the top-level xact contains any catalog change and it is present in the initial running xacts array. This fix is required only for 14 and 15 because in prior branches we already always mark the transaction and all its subtransactions as containing catalog changes in the same case. In 16 and above, we preserve the list of transaction IDs and sub-transaction IDs, that have modified catalogs and are running during snapshot serialization, to the serialized snapshot (see commit 7f13ac8123). Author: Fei Changhong Reviewed-by: Amit Kapila, Hayato Kuroda, Andy Fan Discussion: https://postgr.es/m/18280-4c8060178cb41750@postgresql.org	2 years ago
Alvaro Herrera	3bb8b9342f	Make test_decoding ddl.out shorter ... Some of the test_decoding test output was extremely wide, because it deals with massive toasted values, and the aligned mode causes psql to produce 200kB of whitespace and dashes. Change to unaligned mode temporarily to avoid that behavior. Backpatch to 14, where it applies cleanly. Discussion: https://postgr.es/m/20230405103953.sxleixp3uz5lazst@alvherre.pgsql	3 years ago
Amit Kapila	343afa9671	Fix executing invalidation messages generated by subtransactions during decoding. ... This problem has been introduced by commit 272248a0c1 where we started assigning the subtransactions to the top-level transaction when we mark both the top-level transaction and its subtransactions as containing catalog changes. After we assign subtransactions to the top-level transaction, we were not allowed to execute any invalidations associated with it when we decide to skip the transaction. The reason to assign the subtransactions to the top-level transaction was to avoid the assertion failure in AssertTXNLsnOrder() as they have the same LSN when we sometimes start accumulating transaction changes for partial transactions after the restart. Now that with commit 64ff0fe4e8, we skip this assertion check until we reach the LSN at which we start decoding the contents of the transaction, so, there is no reason for such an assignment anymore. The assignment change was introduced in 15 and prior versions but this bug doesn't exist in branches prior to 14 since we don't add invalidation messages to subtransactions. We decided to backpatch through 11 for consistency but not for 10 since its final release is near. Reported-by: Kuroda Hayato Author: Masahiko Sawada Reviewed-by: Amit Kapila Backpatch-through: 11 Discussion: https://postgr.es/m/TYAPR01MB58660803BCAA7849C8584AA4F57E9%40TYAPR01MB5866.jpnprd01.prod.outlook.com Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com	3 years ago
Amit Kapila	64ff0fe4e8	Fix assertion failures while processing NEW_CID record in logical decoding. ... When the logical decoding restarts from NEW_CID, since there is no association between the top transaction and its subtransaction, both are created as top transactions and have the same LSN. This caused the assertion failure in AssertTXNLsnOrder(). This patch skips the assertion check until we reach the LSN at which we start decoding the contents of the transaction, specifically start_decoding_at LSN in SnapBuild. This is okay because we don't guarantee to make the association between top transaction and subtransaction until we try to decode the actual contents of transaction. The ordering of the records prior to the start_decoding_at LSN should have been checked before the restart. The other assertion failure is due to the reason that we forgot to track that we have considered top-level transaction id in the list of catalog changing transactions that were committed when one of its subtransactions is marked as containing catalog change. Reported-by: Tomas Vondra, Osumi Takamichi Author: Masahiko Sawada, Kuroda Hayato Reviewed-by: Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi, Masahiko Sawada Backpatch-through: 10 Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com Discussion: https://postgr.es/m/TYCPR01MB83733C6CEAE47D0280814D5AED7A9%40TYCPR01MB8373.jpnprd01.prod.outlook.com	3 years ago
Amit Kapila	272248a0c1	Fix catalog lookup with the wrong snapshot during logical decoding. ... Previously, we relied on HEAP2_NEW_CID records and XACT_INVALIDATION records to know if the transaction has modified the catalog, and that information is not serialized to snapshot. Therefore, after the restart, if the logical decoding decodes only the commit record of the transaction that has actually modified a catalog, we will miss adding its XID to the snapshot. Thus, we will end up looking at catalogs with the wrong snapshot. To fix this problem, this changes the snapshot builder so that it remembers the last-running-xacts list of the decoded RUNNING_XACTS record after restoring the previously serialized snapshot. Then, we mark the transaction as containing catalog changes if it's in the list of initial running transactions and its commit record has XACT_XINFO_HAS_INVALS. To avoid ABI breakage, we store the array of the initial running transactions in the static variables InitialRunningXacts and NInitialRunningXacts, instead of storing those in SnapBuild or ReorderBuffer. This approach has a false positive; we could end up adding the transaction that didn't change catalog to the snapshot since we cannot distinguish whether the transaction has catalog changes only by checking the COMMIT record. It doesn't have the information on which (sub) transaction has catalog changes, and XACT_XINFO_HAS_INVALS doesn't necessarily indicate that the transaction has catalog change. But that won't be a problem since we use snapshot built during decoding only to read system catalogs. On the master branch, we took a more future-proof approach by writing catalog modifying transactions to the serialized snapshot which avoids the above false positive. But we cannot backpatch it because of a change in the SnapBuild. Reported-by: Mike Oh Author: Masahiko Sawada Reviewed-by: Amit Kapila, Shi yu, Takamichi Osumi, Kyotaro Horiguchi, Bertrand Drouvot, Ahsan Hadi Backpatch-through: 10 Discussion: https://postgr.es/m/81D0D8B0-E7C4-4999-B616-1E5004DBDCD2%40amazon.com	4 years ago
Andres Freund	5264add784	pgstat: add/extend tests for resetting various kinds of stats. ... - subscriber stats reset path was untested - slot stat sreset path for all slots was untested - pg_stat_database.sessions etc was untested - pg_stat_reset_shared() was untested, for any kind of shared stats - pg_stat_reset() was untested Author: Melanie Plageman <melanieplageman@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20220303021600.hs34ghqcw6zcokdh@alap3.anarazel.de	4 years ago
Tomas Vondra	2c7ea57e56	Revert "Logical decoding of sequences" ... This reverts a sequence of commits, implementing features related to logical decoding and replication of sequences: - 0da92dc530 - 80901b3291 - b779d7d8fd - d5ed9da41d - a180c2b34d - 75b1521dae - 2d2232933b - 002c9dd97a - 05843b1aa4 The implementation has issues, mostly due to combining transactional and non-transactional behavior of sequences. It's not clear how this could be fixed, but it'll require reworking significant part of the patch. Discussion: https://postgr.es/m/95345a19-d508-63d1-860a-f5c2f41e8d40@enterprisedb.com	4 years ago
Andres Freund	0f96965c65	pgstat: add pg_stat_force_next_flush(), use it to simplify tests. ... In the stats collector days it was hard to write tests for the stats system, because fundamentally delivery of stats messages over UDP was not synchronous (nor guaranteed). Now we easily can force pending stats updates to be flushed synchronously. This moves stats.sql into a parallel group, there isn't a reason for it to run in isolation anymore. And it may shake out some bugs. Bumps catversion. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20220303021600.hs34ghqcw6zcokdh@alap3.anarazel.de	4 years ago
Alvaro Herrera	7103ebb7aa	Add support for MERGE SQL command ... MERGE performs actions that modify rows in the target table using a source table or query. MERGE provides a single SQL statement that can conditionally INSERT/UPDATE/DELETE rows -- a task that would otherwise require multiple PL statements. For example, MERGE INTO target AS t USING source AS s ON t.tid = s.sid WHEN MATCHED AND t.balance > s.delta THEN UPDATE SET balance = t.balance - s.delta WHEN MATCHED THEN DELETE WHEN NOT MATCHED AND s.delta > 0 THEN INSERT VALUES (s.sid, s.delta) WHEN NOT MATCHED THEN DO NOTHING; MERGE works with regular tables, partitioned tables and inheritance hierarchies, including column and row security enforcement, as well as support for row and statement triggers and transition tables therein. MERGE is optimized for OLTP and is parameterizable, though also useful for large scale ETL/ELT. MERGE is not intended to be used in preference to existing single SQL commands for INSERT, UPDATE or DELETE since there is some overhead. MERGE can be used from PL/pgSQL. MERGE does not support targetting updatable views or foreign tables, and RETURNING clauses are not allowed either. These limitations are likely fixable with sufficient effort. Rewrite rules are also not supported, but it's not clear that we'd want to support them. Author: Pavan Deolasee <pavan.deolasee@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Author: Amit Langote <amitlangote09@gmail.com> Author: Simon Riggs <simon.riggs@enterprisedb.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: Andres Freund <andres@anarazel.de> (earlier versions) Reviewed-by: Peter Geoghegan <pg@bowt.ie> (earlier versions) Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier versions) Reviewed-by: Japin Li <japinli@hotmail.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CANP8+jKitBSrB7oTgT9CY2i1ObfOt36z0XMraQc+Xrz8QB0nXA@mail.gmail.com Discussion: https://postgr.es/m/CAH2-WzkJdBuxj9PO=2QaO9-3h3xGbQPZ34kJH=HukRekwM-GZg@mail.gmail.com Discussion: https://postgr.es/m/20201231134736.GA25392@alvherre.pgsql	4 years ago
Noah Misch	e186f56f9c	Close race condition in slot_creation_error.spec. ... Use the pattern from detach-partition-concurrently-3.spec. Per buildfarm member wrasse. Reviewed by Kyotaro Horiguchi and Andres Freund. Discussion: https://postgr.es/m/20220318072837.GC2739027@rfd.leadboat.com	4 years ago
Tomas Vondra	a180c2b34d	Stabilize test_decoding touching with sequences ... Some of the test_decoding regression tests are unstable due to modifying a sequence. The first increment of a sequence after a checkpoint is always logged (and thus decoded), which makes the output unpredictable. The runs are usually much shorter than a checkpoint internal, so these failures are rare, but we've seen a couple of them on animals that are either slow or are running with valgrind/clobber cache/... Fixed by skipping sequence decoding in most tests, with the exception of the test aimed at testing decoding of sequences. Reported-by: Amita Kapila Discussion: https://postgr.es/m/d045f3c2-6cfb-06d3-5540-e63c320df8bc@enterprisedb.com	4 years ago
Noah Misch	766075105c	Use PG_TEST_TIMEOUT_DEFAULT for pg_regress suite non-elapsing timeouts. ... Currently, only contrib/test_decoding has this property. Use \getenv to load the timeout value. Discussion: https://postgr.es/m/20220218052842.GA3627003@rfd.leadboat.com	4 years ago
Tom Lane	618c16707a	Rearrange libpq's error reporting to avoid duplicated error text. ... Since commit ffa2e4670, libpq accumulates text in conn->errorMessage across a whole query cycle. In some situations, we may report more than one error event within a cycle: the easiest case to reach is where we report a FATAL error message from the server, and then a bit later we detect loss of connection. Since, historically, each error PGresult bears the entire content of conn->errorMessage, this results in duplication of the FATAL message in any output that concatenates the contents of the PGresults. Accumulation in errorMessage still seems like a good idea, especially in view of the number of places that did ad-hoc error concatenation before ffa2e4670. So to fix this, let's track how much of conn->errorMessage has been read out into error PGresults, and only include new text in later PGresults. The tricky part of that is to be sure that we never discard an error PGresult once made (else we'd risk dropping some text, a problem much worse than duplication). While libpq formerly did that in some code paths, a little bit of rearrangement lets us postpone making an error PGresult at all until we are about to return it. A side benefit of that postponement is that it now becomes practical to return a dummy static PGresult in cases where we hit out-of-memory while trying to manufacture an error PGresult. This eliminates the admittedly-very-rare case where we'd return NULL from PQgetResult, indicating successful query completion, even though what actually happened was an OOM failure. Discussion: https://postgr.es/m/ab4288f8-be5c-57fb-2400-e3e857f53e46@enterprisedb.com	4 years ago
Andres Freund	278cdea6b9	Add isolation test for errors during logical slot creation. ... I didn't include this test in 2f6501fa3c, because I was not sure the error messages for the terminated connection is stable across platforms. But it sounds like it is, and if not, we'd want to do something about the instability anyway... Discussion: https://postgr.es/m/CAD21AoDAeEpAbZEyYJsPZJUmSPaRicVSBObaL7sPaofnKz+9zg@mail.gmail.com	4 years ago
Amit Kapila	5e01001ffb	WAL log unchanged toasted replica identity key attributes. ... Currently, during UPDATE, the unchanged replica identity key attributes are not logged separately because they are getting logged as part of the new tuple. But if they are stored externally then the untoasted values are not getting logged as part of the new tuple and logical replication won't be able to replicate such UPDATEs. So we need to log such attributes as part of the old_key_tuple during UPDATE. Reported-by: Haiying Tang Author: Dilip Kumar and Amit Kapila Reviewed-by: Alvaro Herrera, Haiying Tang, Andres Freund Backpatch-through: 10 Discussion: https://postgr.es/m/OS0PR01MB611342D0A92D4F4BF26C0F47FB229@OS0PR01MB6113.jpnprd01.prod.outlook.com	4 years ago
Tomas Vondra	b779d7d8fd	Fix skip-empty-xacts with sequences in test_decoding ... Regression tests need to use skip-empty-xacts = false, because there might be accidental concurrent activity (like autovacuum), particularly on slow machines. The tests added by 80901b3291 failed to do that in a couple places, triggering occasional failures on buildfarm. Fixing the tests however uncovered a bug in the code, because sequence callbacks did not handle skip-empty-xacts properly. For trasactional increments we need to check/update the xact_wrote_changes flag, and emit the BEGIN if it's the first change in the transaction. Reported-by: Andres Freund Discussion: https://postgr.es/m/20220212220413.b25amklo7t4xb7ni%40alap3.anarazel.de	4 years ago
Tomas Vondra	80901b3291	Add decoding of sequences to test_decoding ... Commit 0da92dc530 improved the logical decoding infrastructure to handle sequences, and did various changes to related parts (WAL logging etc.). But it did not include any implementation of the new callbacks added to OutputPluginCallbacks. This extends test_decoding with two callbacks to decode sequences. The decoding of sequences may be disabled using 'include-sequences', a new option of the output plugin. Author: Tomas Vondra, Cary Huang Reviewed-by: Peter Eisentraut, Hannu Krosing, Andres Freund Discussion: https://postgr.es/m/d045f3c2-6cfb-06d3-5540-e63c320df8bc@enterprisedb.com Discussion: https://postgr.es/m/1710ed7e13b.cd7177461430746.3372264562543607781@highgo.ca	4 years ago
Michael Paquier	ece8c76192	Remove assertion for replication origins in PREPARE TRANSACTION ... When using replication origins, pg_replication_origin_xact_setup() is an optional choice to be able to set a LSN and a timestamp to mark the origin, which would be additionally added to WAL for transaction commits or aborts (including 2PC transactions). An assertion in the code path of PREPARE TRANSACTION assumed that this data should always be set, so it would trigger when using replication origins without setting up an origin LSN. Some tests are added to cover more this kind of scenario. Oversight in commit 1eb6d65. Per discussion with Amit Kapila and Masahiko Sawada. Discussion: https://postgr.es/m/YbbBfNSvMm5nIINV@paquier.xyz Backpatch-through: 11	4 years ago
Michael Paquier	1922d7c6e1	Add SQL functions to monitor the directory contents of replication slots ... This commit adds a set of functions able to look at the contents of various paths related to replication slots: - pg_ls_logicalsnapdir, for pg_logical/snapshots/ - pg_ls_logicalmapdir, for pg_logical/mappings/ - pg_ls_replslotdir, for pg_replslot/<slot_name>/ These are intended to be used by monitoring tools. Unlike pg_ls_dir(), execution permission can be granted to non-superusers. Roles members of pg_monitor gain have access to those functions. Bump catalog version. Author: Bharath Rupireddy Reviewed-by: Nathan Bossart, Justin Pryzby Discussion: https://postgr.es/m/CALj2ACWsfizZjMN6bzzdxOk1ADQQeSw8HhEjhmVXn_Pu+7VzLw@mail.gmail.com	4 years ago
Amit Kapila	29b5905470	Fix toast rewrites in logical decoding. ... Commit 325f2ec555 introduced pg_class.relwrite to skip operations on tables created as part of a heap rewrite during DDL. It links such transient heaps to the original relation OID via this new field in pg_class but forgot to do anything about toast tables. So, logical decoding was not able to skip operations on internally created toast tables. This leads to an error when we tried to decode the WAL for the next operation for which it appeared that there is a toast data where actually it didn't have any toast data. To fix this, we set pg_class.relwrite for internally created toast tables as well which allowed skipping operations on them during logical decoding. Author: Bertrand Drouvot Reviewed-by: David Zhang, Amit Kapila Backpatch-through: 11, where it was introduced Discussion: https://postgr.es/m/b5146fb1-ad9e-7d6e-f980-98ed68744a7c@amazon.com	5 years ago
Tom Lane	4a054069a3	Improve display of query results in isolation tests. ... Previously, isolationtester displayed SQL query results using some ad-hoc code that clearly hadn't had much effort expended on it. Field values longer than 14 characters weren't separated from the next field, and usually caused misalignment of the columns too. Also there was no visual separation of a query's result from subsequent isolationtester output. This made test result files confusing and hard to read. To improve matters, let's use libpq's PQprint() function. Although that's long since unused by psql, it's still plenty good enough for the purpose here. Like 741d7f104, back-patch to all supported branches, so that this isn't a stumbling block for back-patching isolation test changes. Discussion: https://postgr.es/m/582362.1623798221@sss.pgh.pa.us	5 years ago
Tom Lane	741d7f1047	Use annotations to reduce instability of isolation-test results. ... We've long contended with isolation test results that aren't entirely stable. Some test scripts insert long delays to try to force stable results, which is not terribly desirable; but other erratic failure modes remain, causing unrepeatable buildfarm failures. I've spent a fair amount of time trying to solve this by improving the server-side support code, without much success: that way is fundamentally unable to cope with diffs that stem from chance ordering of arrival of messages from different server processes. We can improve matters on the client side, however, by annotating the test scripts themselves to show the desired reporting order of events that might occur in different orders. This patch adds three types of annotations to deal with (a) test steps that might or might not complete their waits before the isolationtester can see them waiting; (b) test steps in different sessions that can legitimately complete in either order; and (c) NOTIFY messages that might arrive before or after the completion of a step in another session. We might need more annotation types later, but this seems to be enough to deal with the instabilities we've seen in the buildfarm. It also lets us get rid of all the long delays that were previously used, cutting more than a minute off the runtime of the isolation tests. Back-patch to all supported branches, because the buildfarm instabilities affect all the branches, and because it seems desirable to keep isolationtester's capabilities the same across all branches to simplify possible future back-patching of tests. Discussion: https://postgr.es/m/327948.1623725828@sss.pgh.pa.us	5 years ago
Amit Kapila	6f4bdf8152	Fix assertion during streaming of multi-insert toast changes. ... While decoding the multi-insert WAL we can't clean the toast untill we get the last insert of that WAL record. Now if we stream the changes before we get the last change, the memory for toast chunks won't be released and we expect the txn to have streamed all changes after streaming. This restriction is mainly to ensure the correctness of streamed transactions and it doesn't seem worth uplifting such a restriction just to allow this case because anyway we will stream the transaction once such an insert is complete. Previously we were using two different flags (one for toast tuples and another for speculative inserts) to indicate partial changes. Now instead we replaced both of them with a single flag to indicate partial changes. Reported-by: Pavan Deolasee Author: Dilip Kumar Reviewed-by: Pavan Deolasee, Amit Kapila Discussion: https://postgr.es/m/CABOikdN-_858zojYN-2tNcHiVTw-nhxPwoQS4quExeweQfG1Ug@mail.gmail.com	5 years ago
Amit Kapila	fc69509131	Fix tests for replication slots stats. ... Some of the tests were not considering that the slot's spill stats could be received by the stats collector after we have reset the stats. Remove those tests and don't check total bytes decoded and sent to output plugin in the spilled stats test as we can send the spilled stats to the stats collector before actually sending the changes to output plugin. Reported-by: Tom Lane as per buildfarm Author: Vignesh C, Sawada Masahiko Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de	5 years ago
Amit Kapila	51ef917303	Another try to fix the test case added by commit f5fc2f5b23. ... As per analysis, it appears that the 'drop slot' message from the previous test and 'create slot' message of the new test are either missed or not yet delivered to the stats collector due to which we will still see the stats from the old slot. This can happen rarely which could be the reason that we are seeing some failures in the buildfarm randomly. To avoid that we are using a different slot name for the tests in test_decoding/sql/stats.sql. Reported-by: Tom Lane based on buildfarm reports Author: Sawada Masahiko Reviewed-by: Amit Kapila, Vignesh C Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de	5 years ago
Amit Kapila	c64dcc7fee	Fix test case added by commit f5fc2f5b23. ... In the new test after resetting the stats, we were not waiting for the stats message to be delivered. Also, we need to decode the results for the new test, otherwise, it will show the old stats. In passing, a. Change docs added by commit f5fc2f5b23 as per suggestion by Justin Pryzby. b. Bump the PGSTAT_FILE_FORMAT_ID as commit f5fc2f5b23 changes the file format of stats. Reported-by: Tom Lane based on buildfarm reports Author: Vignesh C, Justin Pryzby Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de	5 years ago
Amit Kapila	f5fc2f5b23	Add information of total data processed to replication slot stats. ... This adds the statistics about total transactions count and total transaction data logically sent to the decoding output plugin from ReorderBuffer. Users can query the pg_stat_replication_slots view to check these stats. Suggested-by: Andres Freund Author: Vignesh C and Amit Kapila Reviewed-by: Sawada Masahiko, Amit Kapila Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de	5 years ago
Peter Eisentraut	0b5e824528	Message improvement ... The previous wording contained a superfluous comma. Adjust phrasing for grammatical correctness and clarity.	5 years ago
Andres Freund	5f79580ad6	Fix memory lifetime issues of replication slot stats. ... When accessing replication slot stats, introduced in 9868167500, pgstat_read_statsfiles() reads the data into newly allocated memory. Unfortunately the current memory context at that point is the callers, leading to leaks and use-after-free dangers. The fix is trivial, explicitly use pgStatLocalContext. There's some potential for further improvements, but that's outside of the scope of this bugfix. No backpatch necessary, feature is only in HEAD. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20210317230447.c7uc4g3vbs4wi32i@alap3.anarazel.de	5 years ago
Amit Kapila	19890a064e	Add option to enable two_phase commits via pg_create_logical_replication_slot. ... Commit 0aa8a01d04 extends the output plugin API to allow decoding of prepared xacts and allowed the user to enable/disable the two-phase option via pg_logical_slot_get_changes(). This can lead to a problem such that the first time when it gets changes via pg_logical_slot_get_changes() without two_phase option enabled it will not get the prepared even though prepare is after consistent snapshot. Now next time during getting changes, if the two_phase option is enabled it can skip prepare because by that time start decoding point has been moved. So the user will only get commit prepared. Allow to enable/disable this option at the create slot time and default will be false. It will break the existing slots which is fine in a major release. Author: Ajin Cherian Reviewed-by: Amit Kapila and Vignesh C Discussion: https://postgr.es/m/d0f60d60-133d-bf8d-bd70-47784d8fabf3@enterprisedb.com	5 years ago
Amit Kapila	8bdb1332eb	Avoid repeated decoding of prepared transactions after a restart. ... In commit a271a1b50e, we allowed decoding at prepare time and the prepare was decoded again if there is a restart after decoding it. It was done that way because we can't distinguish between the cases where we have not decoded the prepare because it was prior to consistent snapshot or we have decoded it earlier but restarted. To distinguish between these two cases, we have introduced an initial_consistent_point at the slot level which is an LSN at which we found a consistent point at the time of slot creation. This is also the point where we have exported a snapshot for the initial copy. So, prepare transaction prior to this point are sent along with commit prepared. This commit bumps SNAPBUILD_VERSION because of change in SnapBuild. It will break existing slots which is fine in a major release. Author: Ajin Cherian, based on idea by Andres Freund Reviewed-by: Amit Kapila and Vignesh C Discussion: https://postgr.es/m/d0f60d60-133d-bf8d-bd70-47784d8fabf3@enterprisedb.com	5 years ago
Noah Misch	388b959315	Raise a timeout to 180s, in contrib/test_decoding. ... Per buildfarm member hornet. The test is new in v14, so no back-patch.	5 years ago
Amit Kapila	e33d004900	Fix the test for decoding of two-phase transactions. ... Commit 5a3574d7b3 added the test for decoding of two-phase transactions during the build of a consistent snapshot. The test forgot to skip empty xacts which can lead to decoding of extra empty transactions due to background activity by autovacuum. Per report by buildfarm. Reported-by: Tom Lane Discussion: https://postgr.es/m/363512.1610171267@sss.pgh.pa.us	5 years ago
Amit Kapila	5a3574d7b3	Test decoding of two-phase transactions during the build of a consistent snapshot. ... Commit a271a1b50e added the capability to allow decoding at prepare time. This adds an isolation testcase to test that decoding happens at commit time when the consistent snapshot state is reached after prepare but before commit prepared. Author: Ajin Cherian Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com	5 years ago
Amit Kapila	a271a1b50e	Allow decoding at prepare time in ReorderBuffer. ... This patch allows PREPARE-time decoding of two-phase transactions (if the output plugin supports this capability), in which case the transactions are replayed at PREPARE and then committed later when COMMIT PREPARED arrives. Now that we decode the changes before the commit, the concurrent aborts may cause failures when the output plugin consults catalogs (both system and user-defined). We detect such failures with a special sqlerrcode ERRCODE_TRANSACTION_ROLLBACK introduced by commit 7259736a6e and stop decoding the remaining changes. Then we rollback the changes when rollback prepared is encountered. Author: Ajin Cherian and Amit Kapila based on previous work by Nikhil Sontakke and Stas Kelvich Reviewed-by: Amit Kapila, Peter Smith, Sawada Masahiko, Arseny Sher, and Dilip Kumar Tested-by: Takamichi Osumi Discussion: https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com	5 years ago
Amit Kapila	9653f24ad8	Fix 'skip-empty-xacts' option in test_decoding for streaming mode. ... In streaming mode, the transaction can be decoded in multiple streams and those streams can be interleaved with streams of other transactions. So, we can't remember the transaction's write status in the logical decoding context because that might get changed due to some other transactions and lead to wrong answers for 'skip-empty-xacts' option. We decided to keep each transaction's write status in the ReorderBufferTxn to avoid interleaved streams changing the status of some unrelated transactions. Diagnosed-by: Amit Kapila Author: Dilip Kumar Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAA4eK1LR7=XNM_TLmpZMFuV8ZQpoxkem--NZJYf8YXmesbvwLA@mail.gmail.com	5 years ago
Amit Kapila	03d51b776d	Change the attribute name in pg_stat_replication_slots view. ... Change the attribute 'name' to 'slot_name' in pg_stat_replication_slots view to make it clear and that way we will be consistent with the other places like pg_stat_wal_receiver view where we display the same attribute. In the passing, fix the typo in one of the macros in the related code. Bump the catversion as we have modified the name in the catalog as well. Reported-by: Noriyoshi Shinoda Author: Noriyoshi Shinoda Reviewed-by: Sawada Masahiko and Amit Kapila Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com	5 years ago
Amit Kapila	2050832d0d	Fix the unstable output of tests added by commit 8fccf75834. ... The test cases added by that commit were trying to test the exact number of times a particular transaction has spilled. However, that number can vary if any background transaction (say by autovacuum) happens in parallel to the main transaction. So let's not try to verify the exact count. Author: Amit Kapila Reviewed-by: Sawada Masahiko Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com	5 years ago
Amit Kapila	8fccf75834	Add tests for logical replication spilled stats. ... Commit 9868167500 added a mechanism to track statistics corresponding to the spilling of changes from ReorderBuffer but didn't add any tests. Author: Amit Kapila and Sawada Masahiko Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com	5 years ago
Amit Kapila	0ba5181c00	Skip empty transaction stream in test_decoding. ... We were decoding empty transactions via streaming APIs added in commit 45fdc9738b even when the user used the option 'skip-empty-xacts'. The APIs makes no effort to skip empty xacts under the assumption that we will never try to stream such transactions. However, that is not true because we can pick to stream a transaction that has change messages for REORDER_BUFFER_CHANGE_INTERNAL_SNAPSHOT and we don't send such messages to downstream rather they are just to update the internal state. So, we need to skip such xacts when plugin uses the option 'skip-empty-xacts'. Diagnosed-By: Amit Kapila Author: Dilip Kumar Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAA4eK1+OqgFNZkf7=ETe_y5ntjgDk3T0wcdkd4Sot_u1hySGfw@mail.gmail.com	6 years ago
Michael Paquier	fe7fd4e961	Add regression tests for REPLICA IDENTITY with dropped indexes ... REPLICA IDENTITY USING INDEX behaves the same way as NOTHING if the associated index is dropped, even if there is a primary key that could be used as a fallback for the changes generated. There have never been any tests to cover such scenarios, so this commit closes the gap. Author: Michael Paquier Reviewed-by: Masahiko Sawada, Rahila Syed, Euler Taveira Discussion: https://postgr.es/m/20200522035028.GO2355@paquier.xyz	6 years ago
Amit Kapila	82a0ba7707	Fix the logical streaming test. ... Commit 7259736a6e added the capability to stream changes in ReorderBuffer which has some tests to test the streaming mode. It is quite possible that while this test is running a parallel transaction could be logged by autovacuum. Such a transaction won't perform any insert/update/delete to non-catalog tables so will be shown as an empty transaction. Fix it by skipping the empty transactions during this test. Per report by buildfarm.	6 years ago
Amit Kapila	7259736a6e	Implement streaming mode in ReorderBuffer. ... Instead of serializing the transaction to disk after reaching the logical_decoding_work_mem limit in memory, we consume the changes we have in memory and invoke stream API methods added by commit 45fdc9738b. However, sometimes if we have incomplete toast or speculative insert we spill to the disk because we can't generate the complete tuple and stream. And, as soon as we get the complete tuple we stream the transaction including the serialized changes. We can do this incremental processing thanks to having assignments (associating subxact with toplevel xacts) in WAL right away, and thanks to logging the invalidation messages at each command end. These features are added by commits 0bead9af48 and c55040ccd0 respectively. Now that we can stream in-progress transactions, the concurrent aborts may cause failures when the output plugin consults catalogs (both system and user-defined). We handle such failures by returning ERRCODE_TRANSACTION_ROLLBACK sqlerrcode from system table scan APIs to the backend or WALSender decoding a specific uncommitted transaction. The decoding logic on the receipt of such a sqlerrcode aborts the decoding of the current transaction and continue with the decoding of other transactions. We have ReorderBufferTXN pointer in each ReorderBufferChange by which we know which xact it belongs to. The output plugin can use this to decide which changes to discard in case of stream_abort_cb (e.g. when a subxact gets discarded). We also provide a new option via SQL APIs to fetch the changes being streamed. Author: Dilip Kumar, Tomas Vondra, Amit Kapila, Nikhil Sontakke Reviewed-by: Amit Kapila, Kuntal Ghosh, Ajin Cherian Tested-by: Neha Sharma, Mahendra Singh Thalor and Ajin Cherian Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com	6 years ago