public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v7 4/7] Invalidate parent indexes 15+ messages / 5 participants [nested] [flat]
* [PATCH v7 4/7] Invalidate parent indexes @ 2020-11-06 00:58 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Justin Pryzby @ 2020-11-06 00:58 UTC (permalink / raw) --- src/backend/commands/cluster.c | 21 +++++++++++++++++++++ src/test/regress/expected/cluster.out | 26 ++++++++++++++++++++++++++ src/test/regress/sql/cluster.sql | 8 ++++++++ 3 files changed, 55 insertions(+) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 78c2c2ba72..05c84e5582 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -594,6 +594,27 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) } list_free(inh); + /* + * Set parent of all indexes as unclustered when a rel is unclustered; and, + * when an index is clustered, set parents of all /other/ indexes as + * unclustered. + */ + indexes = RelationGetIndexList(rel); + foreach (lc, indexes) + { + Oid thisIndexOid = lfirst_oid(lc); + + if (thisIndexOid == indexOid) + continue; + + while (get_rel_relispartition(thisIndexOid)) + { + thisIndexOid = get_partition_parent(thisIndexOid); + set_indisclustered(thisIndexOid, false, pg_index); + } + } + list_free(indexes); + table_close(pg_index, RowExclusiveLock); } diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 1d436dfaae..6cba3cc4f9 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -541,6 +541,32 @@ Indexes: "clstrpart1_idx_2" btree (a) CLUSTER Number of partitions: 2 (Use \d+ to list them.) +-- Check that the parent index is marked not clustered after clustering a partition on a different index: +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +CLUSTER clstrpart1 USING clstrpart1_idx_2; +\d clstrpart + Partitioned table "public.clstrpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition key: RANGE (a) +Indexes: + "clstrpart_idx" btree (a) +Number of partitions: 3 (Use \d+ to list them.) + +-- Check that the parent index is marked not clustered after setting a partition not clustered: +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +ALTER TABLE clstrpart1 SET WITHOUT CLUSTER; +\d clstrpart + Partitioned table "public.clstrpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition key: RANGE (a) +Indexes: + "clstrpart_idx" btree (a) +Number of partitions: 3 (Use \d+ to list them.) + -- Test CLUSTER with external tuplesorting create table clstr_4 as select * from tenk1; create index cluster_sort on clstr_4 (hundred, thousand, tenthous); diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index 0ded2be1ca..a1d132f288 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -231,6 +231,14 @@ CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a); ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2; \d clstrpart1 +-- Check that the parent index is marked not clustered after clustering a partition on a different index: +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +CLUSTER clstrpart1 USING clstrpart1_idx_2; +\d clstrpart +-- Check that the parent index is marked not clustered after setting a partition not clustered: +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +ALTER TABLE clstrpart1 SET WITHOUT CLUSTER; +\d clstrpart -- Test CLUSTER with external tuplesorting -- 2.17.0 --AA9g+nFNFPYNJKiL Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0005-Invalidate-parent-index-cluster-on-attach.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-22 13:29 Давыдов Виталий <[email protected]> 0 siblings, 2 replies; 15+ messages in thread From: Давыдов Виталий @ 2024-02-22 13:29 UTC (permalink / raw) To: [email protected] Dear All, I'd like to present and talk about a problem when 2PC transactions are applied quite slowly on a replica during logical replication. There is a master and a replica with established logical replication from the master to the replica with twophase = true. With some load level on the master, the replica starts to lag behind the master, and the lag will be increasing. We have to significantly decrease the load on the master to allow replica to complete the catchup. Such problem may create significant difficulties in the production. The problem appears at least on REL_16_STABLE branch. To reproduce the problem: * Setup logical replication from master to replica with subscription parameter twophase = true. * Create some intermediate load on the master (use pgbench with custom sql with prepare+commit) * Optionally switch off the replica for some time (keep load on master). * Switch on the replica and wait until it reaches the master. The replica will never reach the master with even some low load on the master. If to remove the load, the replica will reach the master for much greater time, than expected. I tried the same for regular transactions, but such problem doesn't appear even with a decent load. I think, the main proplem of 2PC catchup bad performance - the lack of asynchronous commit support for 2PC. For regular transactions asynchronous commit is used on the replica by default (subscrition sycnronous_commit = off). It allows the replication worker process on the replica to avoid fsync (XLogFLush) and to utilize 100% CPU (the background wal writer or checkpointer will do fsync). I agree, 2PC are mostly used in multimaster configurations with two or more nodes which are performed synchronously, but when the node in catchup (node is not online in a multimaster cluster), asynchronous commit have to be used to speedup the catchup. There is another thing that affects on the disbalance of the master and replica performance. When the master executes requestes from multiple clients, there is a fsync optimization takes place in XLogFlush. It allows to decrease the number of fsync in case when a number of parallel backends write to the WAL simultaneously. The replica applies received transactions in one thread sequentially, such optimization is not applied. I see some possible solutions: * Implement asyncronous commit for 2PC transactions. * Do some hacking with enableFsync when it is possible. I think, asynchronous commit support for 2PC transactions should significantly increase replica performance and help to solve this problem. I tried to implement it (like for usual transactions) but I've found another problem: 2PC state is stored in WAL on prepare, on commit we have to read 2PC state from WAL but the read is delayed until WAL is flushed by the background wal writer (read LSN should be less than flush LSN). Storing 2PC state in a shared memory (as it proposed earlier) may help. I used the following query to monitor the catchup progress on the master:SELECT sent_lsn, pg_current_wal_lsn() FROM pg_stat_replication; I used the following script for pgbench to the master:SELECT md5(random()::text) as mygid \gset BEGIN; DELETE FROM test WHERE v = pg_backend_pid(); INSERT INTO test(v) SELECT pg_backend_pid(); PREPARE TRANSACTION $$:mygid$$; COMMIT PREPARED $$:mygid$$; What do you think? With best regards, Vitaly Davydov ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-23 03:23 Amit Kapila <[email protected]> parent: Давыдов Виталий <[email protected]> 1 sibling, 1 reply; 15+ messages in thread From: Amit Kapila @ 2024-02-23 03:23 UTC (permalink / raw) To: Давыдов Виталий <[email protected]>; +Cc: [email protected] On Thu, Feb 22, 2024 at 6:59 PM Давыдов Виталий <[email protected]> wrote: > > I'd like to present and talk about a problem when 2PC transactions are applied quite slowly on a replica during logical replication. There is a master and a replica with established logical replication from the master to the replica with twophase = true. With some load level on the master, the replica starts to lag behind the master, and the lag will be increasing. We have to significantly decrease the load on the master to allow replica to complete the catchup. Such problem may create significant difficulties in the production. The problem appears at least on REL_16_STABLE branch. > > To reproduce the problem: > > Setup logical replication from master to replica with subscription parameter twophase = true. > Create some intermediate load on the master (use pgbench with custom sql with prepare+commit) > Optionally switch off the replica for some time (keep load on master). > Switch on the replica and wait until it reaches the master. > > The replica will never reach the master with even some low load on the master. If to remove the load, the replica will reach the master for much greater time, than expected. I tried the same for regular transactions, but such problem doesn't appear even with a decent load. > > I think, the main proplem of 2PC catchup bad performance - the lack of asynchronous commit support for 2PC. For regular transactions asynchronous commit is used on the replica by default (subscrition sycnronous_commit = off). It allows the replication worker process on the replica to avoid fsync (XLogFLush) and to utilize 100% CPU (the background wal writer or checkpointer will do fsync). I agree, 2PC are mostly used in multimaster configurations with two or more nodes which are performed synchronously, but when the node in catchup (node is not online in a multimaster cluster), asynchronous commit have to be used to speedup the catchup. > I don't see we do anything specific for 2PC transactions to make them behave differently than regular transactions with respect to synchronous_commit setting. What makes you think so? Can you pin point the code you are referring to? > There is another thing that affects on the disbalance of the master and replica performance. When the master executes requestes from multiple clients, there is a fsync optimization takes place in XLogFlush. It allows to decrease the number of fsync in case when a number of parallel backends write to the WAL simultaneously. The replica applies received transactions in one thread sequentially, such optimization is not applied. > Right, I think for this we need to implement parallel apply. > I see some possible solutions: > > Implement asyncronous commit for 2PC transactions. > Do some hacking with enableFsync when it is possible. > Can you be a bit more specific about what exactly you have in mind to achieve the above solutions? -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-23 04:52 Ajin Cherian <[email protected]> parent: Давыдов Виталий <[email protected]> 1 sibling, 0 replies; 15+ messages in thread From: Ajin Cherian @ 2024-02-23 04:52 UTC (permalink / raw) To: Давыдов Виталий <[email protected]>; +Cc: [email protected] On Fri, Feb 23, 2024 at 12:29 AM Давыдов Виталий <[email protected]> wrote: > Dear All, > > I'd like to present and talk about a problem when 2PC transactions are > applied quite slowly on a replica during logical replication. There is a > master and a replica with established logical replication from the master > to the replica with twophase = true. With some load level on the master, > the replica starts to lag behind the master, and the lag will be > increasing. We have to significantly decrease the load on the master to > allow replica to complete the catchup. Such problem may create significant > difficulties in the production. The problem appears at least on > REL_16_STABLE branch. > > To reproduce the problem: > > - Setup logical replication from master to replica with subscription > parameter twophase = true. > - Create some intermediate load on the master (use pgbench with custom > sql with prepare+commit) > - Optionally switch off the replica for some time (keep load on > master). > - Switch on the replica and wait until it reaches the master. > > The replica will never reach the master with even some low load on the > master. If to remove the load, the replica will reach the master for much > greater time, than expected. I tried the same for regular transactions, but > such problem doesn't appear even with a decent load. > > > I tried this setup and I do see that the logical subscriber does reach the master in a short time. I'm not sure what I'm missing. I stopped the logical subscriber in between while pgbench was running and then started it again and ran the following: postgres=# SELECT sent_lsn, pg_current_wal_lsn() FROM pg_stat_replication; sent_lsn | pg_current_wal_lsn -----------+-------------------- 0/6793FA0 | 0/6793FA0 <=== caught up (1 row) My pgbench command: pgbench postgres -p 6972 -c 2 -j 3 -f /home/ajin/test.sql -T 200 -P 5 my custom sql file: cat test.sql SELECT md5(random()::text) as mygid \gset BEGIN; DELETE FROM test WHERE v = pg_backend_pid(); INSERT INTO test(v) SELECT pg_backend_pid(); PREPARE TRANSACTION $$:mygid$$; COMMIT PREPARED $$:mygid$$; regards, Ajin Cherian Fujitsu Australia ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-23 17:11 Давыдов Виталий <[email protected]> parent: Amit Kapila <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Давыдов Виталий @ 2024-02-23 17:11 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: [email protected] Hi Amit, Amit Kapila <[email protected]> wrote: I don't see we do anything specific for 2PC transactions to make them behave differently than regular transactions with respect to synchronous_commit setting. What makes you think so? Can you pin point the code you are referring to?Yes, sure. The function RecordTransactionCommitPrepared is called on prepared transaction commit (twophase.c). It calls XLogFlush unconditionally. The function RecordTransactionCommit (for regular transactions, xact.c) calls XLogFlush if synchronous_commit > OFF, otherwise it calls XLogSetAsyncXactLSN. There is some comment in RecordTransactionCommitPrepared (by Bruce Momjian) that shows that async commit is not supported yet: /* * We don't currently try to sleep before flush here ... nor is there any * support for async commit of a prepared xact (the very idea is probably * a contradiction) */ /* Flush XLOG to disk */ XLogFlush(recptr); Right, I think for this we need to implement parallel apply.Yes, parallel apply is a good point. But, I believe, it will not work if asynchronous commit is not supported. You have only one receiver process which should dispatch incoming messages to parallel workers. I guess, you will never reach such rate of parallel execution on replica as on the master with multiple backends. Can you be a bit more specific about what exactly you have in mind to achieve the above solutions?My proposal is to implement async commit for 2PC transactions as it is for regular transactions. It should significantly speedup the catchup process. Then, think how to apply in parallel, which is much diffcult to do. The current problem is to get 2PC state from the WAL on commit prepared. At this moment, the WAL is not flushed yet, commit function waits until WAL with 2PC state is to be flushed. I just tried to do it in my sandbox and found such a problem. Inability to get 2PC state from unflushed WAL stops me right now. I think about possible solutions. The idea with enableFsync is not a suitable solution, in general, I think. I just pointed it as an alternate idea. You just do enableFsync = false before prepare or commit prepared and do enableFsync = true after these functions. In this case, 2PC records will not be fsync-ed, but FlushPtr will be increased. Thus, 2PC state can be read from WAL on commit prepared without waiting. To make it work correctly, I guess, we have to do some additional work to keep more wal on the master and filter some duplicate transactions on the replica, if replica restarts during catchup. With best regards, Vitaly Davydov ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-26 13:24 Amit Kapila <[email protected]> parent: Давыдов Виталий <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Amit Kapila @ 2024-02-26 13:24 UTC (permalink / raw) To: Давыдов Виталий <[email protected]>; +Cc: [email protected] On Fri, Feb 23, 2024 at 10:41 PM Давыдов Виталий <[email protected]> wrote: > > Amit Kapila <[email protected]> wrote: > > I don't see we do anything specific for 2PC transactions to make them behave differently than regular transactions with respect to synchronous_commit setting. What makes you think so? Can you pin point the code you are referring to? > > Yes, sure. The function RecordTransactionCommitPrepared is called on prepared transaction commit (twophase.c). It calls XLogFlush unconditionally. The function RecordTransactionCommit (for regular transactions, xact.c) calls XLogFlush if synchronous_commit > OFF, otherwise it calls XLogSetAsyncXactLSN. > > There is some comment in RecordTransactionCommitPrepared (by Bruce Momjian) that shows that async commit is not supported yet: > /* > * We don't currently try to sleep before flush here ... nor is there any > * support for async commit of a prepared xact (the very idea is probably > * a contradiction) > */ > /* Flush XLOG to disk */ > XLogFlush(recptr); > It seems this comment is added in the commit 4a78cdeb where we added async commit support. I think the reason is probably that when the WAL record for prepared is already flushed then what will be the idea of async commit here? > Right, I think for this we need to implement parallel apply. > > Yes, parallel apply is a good point. But, I believe, it will not work if asynchronous commit is not supported. You have only one receiver process which should dispatch incoming messages to parallel workers. I guess, you will never reach such rate of parallel execution on replica as on the master with multiple backends. > > > Can you be a bit more specific about what exactly you have in mind to achieve the above solutions? > > My proposal is to implement async commit for 2PC transactions as it is for regular transactions. It should significantly speedup the catchup process. Then, think how to apply in parallel, which is much diffcult to do. The current problem is to get 2PC state from the WAL on commit prepared. At this moment, the WAL is not flushed yet, commit function waits until WAL with 2PC state is to be flushed. I just tried to do it in my sandbox and found such a problem. Inability to get 2PC state from unflushed WAL stops me right now. I think about possible solutions. > At commit prepared, it seems we read prepare's WAL record, right? If so, it is not clear to me do you see a problem with a flush of commit_prepared or reading WAL for prepared or both of these. -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-27 11:19 Давыдов Виталий <[email protected]> parent: Amit Kapila <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Давыдов Виталий @ 2024-02-27 11:19 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: [email protected] Hi Amit, Thank you for your interest in the discussion! On Monday, February 26, 2024 16:24 MSK, Amit Kapila <[email protected]> wrote: I think the reason is probably that when the WAL record for prepared is already flushed then what will be the idea of async commit here?I think, the idea of async commit should be applied for both transactions: PREPARE and COMMIT PREPARED, which are actually two separate local transactions. For both these transactions we may call XLogSetAsyncXactLSN on commit instead of XLogFlush when async commit is enabled. When I use async commit, I mean to apply async commit to local transactions, not to a twophase (prepared) transaction itself. At commit prepared, it seems we read prepare's WAL record, right? If so, it is not clear to me do you see a problem with a flush of commit_prepared or reading WAL for prepared or both of these.The problem with reading WAL is due to async commit of PREPARE TRANSACTION which saves 2PC in the WAL. At the moment of COMMIT PREPARED the WAL with PREPARE TRANSACTION 2PC state may not be XLogFlush-ed yet. So, PREPARE TRANSACTION should wait until its 2PC state is flushed. I did some experiments with saving 2PC state in the local memory of logical replication worker and, I think, it worked and demonstrated much better performance. Logical replication worker utilized up to 100% CPU. I'm just concerned about possible problems with async commit for twophase transactions. To be more specific, I've attached a patch to support async commit for twophase. It is not the final patch but it is presented only for discussion purposes. There were some attempts to save 2PC in memory in past but it was rejected. Now, there might be the second round to discuss it. With best regards, Vitaly Attachments: [text/x-patch] 0001-Add-asynchronous-commit-support-for-2PC.patch (5.9K, ../../ba4b6-65ddc580-9-5c3e3080@134362348/3-0001-Add-asynchronous-commit-support-for-2PC.patch) download | inline diff: From 549f809fa122ca0842ec4bfc775afd08feee0d80 Mon Sep 17 00:00:00 2001 From: Vitaly Davydov <[email protected]> Date: Tue, 27 Feb 2024 14:02:23 +0300 Subject: [PATCH] Add asynchronous commit support for 2PC --- src/backend/access/transam/twophase.c | 111 +++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index c6af8cfd7e..52f0853db8 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -109,6 +109,8 @@ #include "utils/memutils.h" #include "utils/timestamp.h" +#define POSTGRESQL_TWOPHASE_SUPPORT_ASYNC_COMMIT + /* * Directory where Two-phase commit files reside within PGDATA */ @@ -163,6 +165,9 @@ typedef struct GlobalTransactionData */ XLogRecPtr prepare_start_lsn; /* XLOG offset of prepare record start */ XLogRecPtr prepare_end_lsn; /* XLOG offset of prepare record end */ + void* prepare_2pc_mem_data; + size_t prepare_2pc_mem_len; + pid_t prepare_2pc_proc; TransactionId xid; /* The GXACT id */ Oid owner; /* ID of user that executed the xact */ @@ -427,6 +432,9 @@ MarkAsPreparing(TransactionId xid, const char *gid, MarkAsPreparingGuts(gxact, xid, gid, prepared_at, owner, databaseid); + Assert(gxact->prepare_2pc_mem_data == NULL); + Assert(gxact->prepare_2pc_proc == 0); + gxact->ondisk = false; /* And insert it into the active array */ @@ -1129,6 +1137,8 @@ StartPrepare(GlobalTransaction gxact) } } +extern bool IsLogicalWorker(void); + /* * Finish preparing state data and writing it to WAL. */ @@ -1167,6 +1177,37 @@ EndPrepare(GlobalTransaction gxact) (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("two-phase state file maximum length exceeded"))); + Assert(gxact->prepare_2pc_mem_data == NULL); + Assert(gxact->prepare_2pc_proc == 0); + + if (IsLogicalWorker()) + { + size_t len = 0; + size_t offset = 0; + + for (record = records.head; record != NULL; record = record->next) + len += record->len; + + if (len > 0) + { + MemoryContext oldmemctx; + + oldmemctx = MemoryContextSwitchTo(TopMemoryContext); + + gxact->prepare_2pc_mem_data = palloc(len); + gxact->prepare_2pc_mem_len = len; + gxact->prepare_2pc_proc = getpid(); + + for (record = records.head; record != NULL; record = record->next) + { + memcpy((char *)gxact->prepare_2pc_mem_data + offset, record->data, record->len); + offset += record->len; + } + + MemoryContextSwitchTo(oldmemctx); + } + } + /* * Now writing 2PC state data to WAL. We let the WAL's CRC protection * cover us, so no need to calculate a separate CRC. @@ -1202,8 +1243,24 @@ EndPrepare(GlobalTransaction gxact) gxact->prepare_end_lsn); } +#if !defined(POSTGRESQL_TWOPHASE_SUPPORT_ASYNC_COMMIT) + XLogFlush(gxact->prepare_end_lsn); +#else + + if (synchronous_commit > SYNCHRONOUS_COMMIT_OFF) + { + /* Flush XLOG to disk */ + XLogFlush(gxact->prepare_end_lsn); + } + else + { + XLogSetAsyncXactLSN(gxact->prepare_end_lsn); + } + +#endif + /* If we crash now, we have prepared: WAL replay will fix things */ /* Store record's start location to read that later on Commit */ @@ -1495,6 +1552,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit) xl_xact_stats_item *commitstats; xl_xact_stats_item *abortstats; SharedInvalidationMessage *invalmsgs; + bool is_local_2pc_buf = false; /* * Validate the GID, and lock the GXACT to ensure that two backends do not @@ -1509,12 +1567,19 @@ FinishPreparedTransaction(const char *gid, bool isCommit) * in WAL files if the LSN is after the last checkpoint record, or moved * to disk if for some reason they have lived for a long time. */ - if (gxact->ondisk) + if (gxact->prepare_2pc_mem_data != NULL && gxact->prepare_2pc_proc == getpid()) + { + Assert(IsLogicalWorker()); + Assert(gxact->prepare_2pc_proc == getpid()); + buf = gxact->prepare_2pc_mem_data; + /* ereport(LOG, errmsg("%s:%d", __FUNCTION__, __LINE__)); */ + is_local_2pc_buf = true; + } + else if (gxact->ondisk) buf = ReadTwoPhaseFile(xid, false); else XlogReadTwoPhaseData(gxact->prepare_start_lsn, &buf, NULL); - /* * Disassemble the header area */ @@ -1638,6 +1703,19 @@ FinishPreparedTransaction(const char *gid, bool isCommit) /* Clear shared memory state */ RemoveGXact(gxact); + if (gxact->prepare_2pc_mem_data != NULL) + { + MemoryContext memctx; + //Assert(gxact->prepare_2pc_proc == getpid()); + memctx = MemoryContextSwitchTo(TopMemoryContext); + if (gxact->prepare_2pc_proc == getpid()) + pfree(gxact->prepare_2pc_mem_data); + gxact->prepare_2pc_mem_data = NULL; + gxact->prepare_2pc_mem_len = 0; + gxact->prepare_2pc_proc = 0; + MemoryContextSwitchTo(memctx); + } + /* * Release the lock as all callbacks are called and shared memory cleanup * is done. @@ -1657,7 +1735,8 @@ FinishPreparedTransaction(const char *gid, bool isCommit) RESUME_INTERRUPTS(); - pfree(buf); + if (!is_local_2pc_buf) + pfree(buf); } /* @@ -2349,12 +2428,38 @@ RecordTransactionCommitPrepared(TransactionId xid, * a contradiction) */ +#if !defined(POSTGRESQL_TWOPHASE_SUPPORT_ASYNC_COMMIT) + /* Flush XLOG to disk */ XLogFlush(recptr); /* Mark the transaction committed in pg_xact */ TransactionIdCommitTree(xid, nchildren, children); +#else + + if (synchronous_commit > SYNCHRONOUS_COMMIT_OFF) + { + /* Flush XLOG to disk */ + XLogFlush(recptr); + + /* Mark the transaction committed in pg_xact */ + TransactionIdCommitTree(xid, nchildren, children); + } + else + { + XLogSetAsyncXactLSN(recptr); + + /* + * We must not immediately update the CLOG, since we didn't flush the + * XLOG. Instead, we store the LSN up to which the XLOG must be + * flushed before the CLOG may be updated. + */ + TransactionIdAsyncCommitTree(xid, nchildren, children, recptr); + } + +#endif + /* Checkpoint can proceed now */ MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; -- 2.34.1 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-27 13:00 Amit Kapila <[email protected]> parent: Давыдов Виталий <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Amit Kapila @ 2024-02-27 13:00 UTC (permalink / raw) To: Давыдов Виталий <[email protected]>; +Cc: [email protected] On Tue, Feb 27, 2024 at 4:49 PM Давыдов Виталий <[email protected]> wrote: > > Thank you for your interest in the discussion! > > On Monday, February 26, 2024 16:24 MSK, Amit Kapila <[email protected]> wrote: > > > I think the reason is probably that when the WAL record for prepared is already flushed then what will be the idea of async commit here? > > I think, the idea of async commit should be applied for both transactions: PREPARE and COMMIT PREPARED, which are actually two separate local transactions. For both these transactions we may call XLogSetAsyncXactLSN on commit instead of XLogFlush when async commit is enabled. When I use async commit, I mean to apply async commit to local transactions, not to a twophase (prepared) transaction itself. > > > At commit prepared, it seems we read prepare's WAL record, right? If so, it is not clear to me do you see a problem with a flush of commit_prepared or reading WAL for prepared or both of these. > > The problem with reading WAL is due to async commit of PREPARE TRANSACTION which saves 2PC in the WAL. At the moment of COMMIT PREPARED the WAL with PREPARE TRANSACTION 2PC state may not be XLogFlush-ed yet. > As we do XLogFlush() at the time of prepare then why it is not available? OR are you talking about this state after your idea/patch where you are trying to make both Prepare and Commit_prepared records async? So, PREPARE TRANSACTION should wait until its 2PC state is flushed. > > I did some experiments with saving 2PC state in the local memory of logical replication worker and, I think, it worked and demonstrated much better performance. Logical replication worker utilized up to 100% CPU. I'm just concerned about possible problems with async commit for twophase transactions. > > To be more specific, I've attached a patch to support async commit for twophase. It is not the final patch but it is presented only for discussion purposes. There were some attempts to save 2PC in memory in past but it was rejected. > It would be good if you could link those threads. -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-27 13:34 Давыдов Виталий <[email protected]> parent: Amit Kapila <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Давыдов Виталий @ 2024-02-27 13:34 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: [email protected] Hi Amit, On Tuesday, February 27, 2024 16:00 MSK, Amit Kapila <[email protected]> wrote: As we do XLogFlush() at the time of prepare then why it is not available? OR are you talking about this state after your idea/patch where you are trying to make both Prepare and Commit_prepared records async?Right, I'm talking about my patch where async commit is implemented. There is no such problem with reading 2PC from not flushed WAL in the vanilla because XLogFlush is called unconditionally, as you've described. But an attempt to add some async stuff leads to the problem of reading not flushed WAL. It is why I store 2pc state in the local memory in my patch. It would be good if you could link those threads.Sure, I will find and add some links to the discussions from past. Thank you! With best regards, Vitaly On Tue, Feb 27, 2024 at 4:49 PM Давыдов Виталий <[email protected]> wrote: > > Thank you for your interest in the discussion! > > On Monday, February 26, 2024 16:24 MSK, Amit Kapila <[email protected]> wrote: > > > I think the reason is probably that when the WAL record for prepared is already flushed then what will be the idea of async commit here? > > I think, the idea of async commit should be applied for both transactions: PREPARE and COMMIT PREPARED, which are actually two separate local transactions. For both these transactions we may call XLogSetAsyncXactLSN on commit instead of XLogFlush when async commit is enabled. When I use async commit, I mean to apply async commit to local transactions, not to a twophase (prepared) transaction itself. > > > At commit prepared, it seems we read prepare's WAL record, right? If so, it is not clear to me do you see a problem with a flush of commit_prepared or reading WAL for prepared or both of these. > > The problem with reading WAL is due to async commit of PREPARE TRANSACTION which saves 2PC in the WAL. At the moment of COMMIT PREPARED the WAL with PREPARE TRANSACTION 2PC state may not be XLogFlush-ed yet. > As we do XLogFlush() at the time of prepare then why it is not available? OR are you talking about this state after your idea/patch where you are trying to make both Prepare and Commit_prepared records async? So, PREPARE TRANSACTION should wait until its 2PC state is flushed. > > I did some experiments with saving 2PC state in the local memory of logical replication worker and, I think, it worked and demonstrated much better performance. Logical replication worker utilized up to 100% CPU. I'm just concerned about possible problems with async commit for twophase transactions. > > To be more specific, I've attached a patch to support async commit for twophase. It is not the final patch but it is presented only for discussion purposes. There were some attempts to save 2PC in memory in past but it was rejected. > It would be good if you could link those threads. -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-02-29 17:34 Давыдов Виталий <[email protected]> parent: Давыдов Виталий <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Давыдов Виталий @ 2024-02-29 17:34 UTC (permalink / raw) To: [email protected] Dear All, Consider, please, my patch for async commit for twophase transactions. It can be applicable when catchup performance is not enought with publication parameter twophase = on. The key changes are: * Use XLogSetAsyncXactLSN instead of XLogFlush as it is for usual transactions. * In case of async commit only, save 2PC state in the pg_twophase file (but not fsync it) in addition to saving in the WAL. The file is used as an alternative to storing 2pc state in the memory. * On recovery, reject pg_twophase files with future xids.Probably, 2PC async commit should be enabled by a GUC (not implemented in the patch). With best regards, Vitaly Attachments: [text/x-patch] 0001-Async-commit-support-for-twophase-transactions.patch (11.6K, ../../cedbe-65e0c000-1-6db17700@133269862/3-0001-Async-commit-support-for-twophase-transactions.patch) download | inline diff: From cbaaa7270d771f9ccd6def08f0f02ce61dc15ff6 Mon Sep 17 00:00:00 2001 From: Vitaly Davydov <[email protected]> Date: Thu, 29 Feb 2024 18:58:13 +0300 Subject: [PATCH] Async commit support for twophase transactions --- src/backend/access/transam/twophase.c | 171 +++++++++++++++++++++----- 1 file changed, 138 insertions(+), 33 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 234c8d08eb..352266be14 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -109,6 +109,8 @@ #include "utils/memutils.h" #include "utils/timestamp.h" +#define POSTGRESQL_TWOPHASE_SUPPORT_ASYNC_COMMIT + /* * Directory where Two-phase commit files reside within PGDATA */ @@ -169,6 +171,7 @@ typedef struct GlobalTransactionData BackendId locking_backend; /* backend currently working on the xact */ bool valid; /* true if PGPROC entry is in proc array */ bool ondisk; /* true if prepare state file is on disk */ + bool infile; /* true if prepared state saved in file (but not fsync-ed) */ bool inredo; /* true if entry was added via xlog_redo */ char gid[GIDSIZE]; /* The GID assigned to the prepared xact */ } GlobalTransactionData; @@ -227,12 +230,14 @@ static void RemoveGXact(GlobalTransaction gxact); static void XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len); static char *ProcessTwoPhaseBuffer(TransactionId xid, XLogRecPtr prepare_start_lsn, - bool fromdisk, bool setParent, bool setNextXid); + bool fromdisk, bool setParent, bool setNextXid, + const char *filename); static void MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, TimestampTz prepared_at, Oid owner, Oid databaseid); static void RemoveTwoPhaseFile(TransactionId xid, bool giveWarning); -static void RecreateTwoPhaseFile(TransactionId xid, void *content, int len); +static void RemoveTwoPhaseFileByName(const char *filename, bool giveWarning); +static void RecreateTwoPhaseFile(TransactionId xid, void *content, int len, bool dosync); /* * Initialization of shared memory @@ -427,6 +432,7 @@ MarkAsPreparing(TransactionId xid, const char *gid, MarkAsPreparingGuts(gxact, xid, gid, prepared_at, owner, databaseid); + gxact->infile = false; gxact->ondisk = false; /* And insert it into the active array */ @@ -1204,6 +1210,37 @@ EndPrepare(GlobalTransaction gxact) (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("two-phase state file maximum length exceeded"))); +#ifdef POSTGRESQL_TWOPHASE_SUPPORT_ASYNC_COMMIT + + Assert(gxact->infile == false); + + if (synchronous_commit == SYNCHRONOUS_COMMIT_OFF) + { + char *buf; + size_t len = 0; + size_t offset = 0; + + for (record = records.head; record != NULL; record = record->next) + len += record->len; + + if (len > 0) + { + buf = palloc(len); + + for (record = records.head; record != NULL; record = record->next) + { + memcpy(buf + offset, record->data, record->len); + offset += record->len; + } + + RecreateTwoPhaseFile(gxact->xid, buf, len, false); + pfree(buf); + gxact->infile = true; + } + } + +#endif + /* * Now writing 2PC state data to WAL. We let the WAL's CRC protection * cover us, so no need to calculate a separate CRC. @@ -1239,8 +1276,24 @@ EndPrepare(GlobalTransaction gxact) gxact->prepare_end_lsn); } +#if !defined(POSTGRESQL_TWOPHASE_SUPPORT_ASYNC_COMMIT) + XLogFlush(gxact->prepare_end_lsn); +#else + + if (synchronous_commit > SYNCHRONOUS_COMMIT_OFF) + { + /* Flush XLOG to disk */ + XLogFlush(gxact->prepare_end_lsn); + } + else + { + XLogSetAsyncXactLSN(gxact->prepare_end_lsn); + } + +#endif + /* If we crash now, we have prepared: WAL replay will fix things */ /* Store record's start location to read that later on Commit */ @@ -1546,12 +1599,11 @@ FinishPreparedTransaction(const char *gid, bool isCommit) * in WAL files if the LSN is after the last checkpoint record, or moved * to disk if for some reason they have lived for a long time. */ - if (gxact->ondisk) + if (gxact->infile || gxact->ondisk) buf = ReadTwoPhaseFile(xid, false); else XlogReadTwoPhaseData(gxact->prepare_start_lsn, &buf, NULL); - /* * Disassemble the header area */ @@ -1687,7 +1739,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit) /* * And now we can clean up any files we may have left. */ - if (gxact->ondisk) + if (gxact->infile || gxact->ondisk) RemoveTwoPhaseFile(xid, true); MyLockedGxact = NULL; @@ -1741,6 +1793,20 @@ RemoveTwoPhaseFile(TransactionId xid, bool giveWarning) errmsg("could not remove file \"%s\": %m", path))); } +static void +RemoveTwoPhaseFileByName(const char *filename, bool giveWarning) +{ + char path[MAXPGPATH]; + + snprintf(path, MAXPGPATH, TWOPHASE_DIR "/%s", filename); + + if (unlink(path)) + if (errno != ENOENT || giveWarning) + ereport(WARNING, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); +} + /* * Recreates a state file. This is used in WAL replay and during * checkpoint creation. @@ -1748,7 +1814,7 @@ RemoveTwoPhaseFile(TransactionId xid, bool giveWarning) * Note: content and len don't include CRC. */ static void -RecreateTwoPhaseFile(TransactionId xid, void *content, int len) +RecreateTwoPhaseFile(TransactionId xid, void *content, int len, bool dosync) { char path[MAXPGPATH]; pg_crc32c statefile_crc; @@ -1791,16 +1857,19 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len) } pgstat_report_wait_end(); - /* - * We must fsync the file because the end-of-replay checkpoint will not do - * so, there being no GXACT in shared memory yet to tell it to. - */ - pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_SYNC); - if (pg_fsync(fd) != 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not fsync file \"%s\": %m", path))); - pgstat_report_wait_end(); + if (dosync) + { + /* + * We must fsync the file because the end-of-replay checkpoint will not do + * so, there being no GXACT in shared memory yet to tell it to. + */ + pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_SYNC); + if (pg_fsync(fd) != 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not fsync file \"%s\": %m", path))); + pgstat_report_wait_end(); + } if (CloseTransientFile(fd) != 0) ereport(ERROR, @@ -1871,7 +1940,8 @@ CheckPointTwoPhase(XLogRecPtr redo_horizon) int len; XlogReadTwoPhaseData(gxact->prepare_start_lsn, &buf, &len); - RecreateTwoPhaseFile(gxact->xid, buf, len); + RecreateTwoPhaseFile(gxact->xid, buf, len, true); + gxact->infile = true; gxact->ondisk = true; gxact->prepare_start_lsn = InvalidXLogRecPtr; gxact->prepare_end_lsn = InvalidXLogRecPtr; @@ -1930,7 +2000,8 @@ restoreTwoPhaseData(void) xid = XidFromFullTransactionId(fxid); buf = ProcessTwoPhaseBuffer(xid, InvalidXLogRecPtr, - true, false, false); + true, false, false, + clde->d_name); if (buf == NULL) continue; @@ -1997,7 +2068,8 @@ PrescanPreparedTransactions(TransactionId **xids_p, int *nxids_p) buf = ProcessTwoPhaseBuffer(xid, gxact->prepare_start_lsn, - gxact->ondisk, false, true); + gxact->ondisk, false, true, + NULL); if (buf == NULL) continue; @@ -2072,7 +2144,8 @@ StandbyRecoverPreparedTransactions(void) buf = ProcessTwoPhaseBuffer(xid, gxact->prepare_start_lsn, - gxact->ondisk, false, false); + gxact->ondisk, false, false, + NULL); if (buf != NULL) pfree(buf); } @@ -2124,7 +2197,8 @@ RecoverPreparedTransactions(void) */ buf = ProcessTwoPhaseBuffer(xid, gxact->prepare_start_lsn, - gxact->ondisk, true, false); + gxact->ondisk, true, false, + NULL); if (buf == NULL) continue; @@ -2202,7 +2276,8 @@ static char * ProcessTwoPhaseBuffer(TransactionId xid, XLogRecPtr prepare_start_lsn, bool fromdisk, - bool setParent, bool setNextXid) + bool setParent, bool setNextXid, + const char *filename) { FullTransactionId nextXid = TransamVariables->nextXid; TransactionId origNextXid = XidFromFullTransactionId(nextXid); @@ -2216,40 +2291,43 @@ ProcessTwoPhaseBuffer(TransactionId xid, if (!fromdisk) Assert(prepare_start_lsn != InvalidXLogRecPtr); - /* Already processed? */ - if (TransactionIdDidCommit(xid) || TransactionIdDidAbort(xid)) + /* Reject XID if too new */ + if (TransactionIdFollowsOrEquals(xid, origNextXid)) { if (fromdisk) { ereport(WARNING, - (errmsg("removing stale two-phase state file for transaction %u", + (errmsg("removing future two-phase state file for transaction %u", xid))); - RemoveTwoPhaseFile(xid, true); + if (filename) + RemoveTwoPhaseFileByName(filename, true); + else + RemoveTwoPhaseFile(xid, true); } else { ereport(WARNING, - (errmsg("removing stale two-phase state from memory for transaction %u", + (errmsg("removing future two-phase state from memory for transaction %u", xid))); PrepareRedoRemove(xid, true); } return NULL; } - /* Reject XID if too new */ - if (TransactionIdFollowsOrEquals(xid, origNextXid)) + /* Already processed? */ + if (TransactionIdDidCommit(xid) || TransactionIdDidAbort(xid)) { if (fromdisk) { ereport(WARNING, - (errmsg("removing future two-phase state file for transaction %u", + (errmsg("removing stale two-phase state file for transaction %u", xid))); RemoveTwoPhaseFile(xid, true); } else { ereport(WARNING, - (errmsg("removing future two-phase state from memory for transaction %u", + (errmsg("removing stale two-phase state from memory for transaction %u", xid))); PrepareRedoRemove(xid, true); } @@ -2388,12 +2466,38 @@ RecordTransactionCommitPrepared(TransactionId xid, * a contradiction) */ +#if !defined(POSTGRESQL_TWOPHASE_SUPPORT_ASYNC_COMMIT) + /* Flush XLOG to disk */ XLogFlush(recptr); /* Mark the transaction committed in pg_xact */ TransactionIdCommitTree(xid, nchildren, children); +#else + + if (synchronous_commit > SYNCHRONOUS_COMMIT_OFF) + { + /* Flush XLOG to disk */ + XLogFlush(recptr); + + /* Mark the transaction committed in pg_xact */ + TransactionIdCommitTree(xid, nchildren, children); + } + else + { + XLogSetAsyncXactLSN(recptr); + + /* + * We must not immediately update the CLOG, since we didn't flush the + * XLOG. Instead, we store the LSN up to which the XLOG must be + * flushed before the CLOG may be updated. + */ + TransactionIdAsyncCommitTree(xid, nchildren, children, recptr); + } + +#endif + /* Checkpoint can proceed now */ MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; @@ -2567,6 +2671,7 @@ PrepareRedoAdd(char *buf, XLogRecPtr start_lsn, gxact->locking_backend = InvalidBackendId; gxact->valid = false; gxact->ondisk = XLogRecPtrIsInvalid(start_lsn); + gxact->infile = gxact->ondisk; gxact->inredo = true; /* yes, added in redo */ strcpy(gxact->gid, gid); @@ -2625,7 +2730,7 @@ PrepareRedoRemove(TransactionId xid, bool giveWarning) * And now we can clean up any files we may have left. */ elog(DEBUG2, "removing 2PC data for transaction %u", xid); - if (gxact->ondisk) + if (gxact->infile || gxact->ondisk) RemoveTwoPhaseFile(xid, giveWarning); RemoveGXact(gxact); } @@ -2673,7 +2778,7 @@ LookupGXact(const char *gid, XLogRecPtr prepare_end_lsn, * do this optimization if we encounter many collisions in GID * between publisher and subscriber. */ - if (gxact->ondisk) + if (gxact->infile || gxact->ondisk) buf = ReadTwoPhaseFile(gxact->xid, false); else { -- 2.34.1 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-03-05 09:05 Heikki Linnakangas <[email protected]> parent: Давыдов Виталий <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Heikki Linnakangas @ 2024-03-05 09:05 UTC (permalink / raw) To: Давыдов Виталий <[email protected]>; [email protected] On 29/02/2024 19:34, Давыдов Виталий wrote: > Dear All, > > Consider, please, my patch for async commit for twophase transactions. > It can be applicable when catchup performance is not enought with > publication parameter twophase = on. > > The key changes are: > > * Use XLogSetAsyncXactLSN instead of XLogFlush as it is for usual > transactions. > * In case of async commit only, save 2PC state in the pg_twophase file > (but not fsync it) in addition to saving in the WAL. The file is > used as an alternative to storing 2pc state in the memory. > * On recovery, reject pg_twophase files with future xids. > > Probably, 2PC async commit should be enabled by a GUC (not implemented > in the patch). In a nutshell, this changes PREPARE TRANSACTION so that if synchronous_commit is 'off', the PREPARE TRANSACTION is not fsync'd to disk. So if you crash after the PREPARE TRANSACTION has returned, the transaction might be lost. I think that's completely unacceptable. If you're ok to lose the prepared state of twophase transactions on crash, why don't you create the subscription with 'two_phase=off' to begin with? -- Heikki Linnakangas Neon (https://neon.tech) ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-03-05 14:29 Давыдов Виталий <[email protected]> parent: Heikki Linnakangas <[email protected]> 0 siblings, 2 replies; 15+ messages in thread From: Давыдов Виталий @ 2024-03-05 14:29 UTC (permalink / raw) To: Heikki Linnakangas <[email protected]>; +Cc: [email protected] Hi Heikki, Thank you for the reply. On Tuesday, March 05, 2024 12:05 MSK, Heikki Linnakangas <[email protected]> wrote: In a nutshell, this changes PREPARE TRANSACTION so that if synchronous_commit is 'off', the PREPARE TRANSACTION is not fsync'd to disk. So if you crash after the PREPARE TRANSACTION has returned, the transaction might be lost. I think that's completely unacceptable. You are right, the prepared transaction might be lost after crash. The same may happen with regular transactions that are not fsync-ed on replica in logical replication by default. The subscription parameter synchronous_commit is OFF by default. I'm not sure, is there some auto recovery for regular transactions? I think, the main difference between these two cases - how to manually recover when some PREPARE TRANSACTION or COMMIT PREPARED are lost. For regular transactions, some updates or deletes in tables on replica may be enough to fix the problem. For twophase transactions, it may be harder to fix it by hands, but it is possible, I believe. If you create a custom solution that is based on twophase transactions (like multimaster) such auto recovery may happen automatically. Another solution is to ignore errors on commit prepared if the corresponding prepared tx is missing. I don't know other risks that may happen with async commit of twophase transactions. If you're ok to lose the prepared state of twophase transactions on crash, why don't you create the subscription with 'two_phase=off' to begin with?In usual work, the subscription has two_phase = on. I have to change this option at catchup stage only, but this parameter can not be altered. There was a patch proposal in past to implement altering of two_phase option, but it was rejected. I think, the recreation of the subscription with two_phase = off will not work. I believe, async commit for twophase transactions on catchup will significantly improve the catchup performance. It is worth to think about such feature. P.S. We might introduce a GUC option to allow async commit for twophase transactions. By default, sync commit will be applied for twophase transactions, as it is now. With best regards, Vitaly Davydov ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-03-06 12:25 Amit Kapila <[email protected]> parent: Давыдов Виталий <[email protected]> 1 sibling, 0 replies; 15+ messages in thread From: Amit Kapila @ 2024-03-06 12:25 UTC (permalink / raw) To: Давыдов Виталий <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; [email protected] On Tue, Mar 5, 2024 at 7:59 PM Давыдов Виталий <[email protected]> wrote: > > Thank you for the reply. > > On Tuesday, March 05, 2024 12:05 MSK, Heikki Linnakangas <[email protected]> wrote: > > > In a nutshell, this changes PREPARE TRANSACTION so that if > synchronous_commit is 'off', the PREPARE TRANSACTION is not fsync'd to > disk. So if you crash after the PREPARE TRANSACTION has returned, the > transaction might be lost. I think that's completely unacceptable. > > > You are right, the prepared transaction might be lost after crash. The same may happen with regular transactions that are not fsync-ed on replica in logical replication by default. The subscription parameter synchronous_commit is OFF by default. I'm not sure, is there some auto recovery for regular transactions? > Unless the commit WAL is not flushed, we wouldn't have updated the replication origin's LSN and neither the walsender would increase the confirmed_flush_lsn for the corresponding slot till the commit is flushed on subscriber. So, if the subscriber crashed before flushing the commit record, server should send the same transaction again. The same should be true for prepared transaction stuff as well. -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-04-04 05:23 Ajin Cherian <[email protected]> parent: Давыдов Виталий <[email protected]> 1 sibling, 1 reply; 15+ messages in thread From: Ajin Cherian @ 2024-04-04 05:23 UTC (permalink / raw) To: Давыдов Виталий <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; [email protected] On Wed, Mar 6, 2024 at 1:29 AM Давыдов Виталий <[email protected]> wrote: > In usual work, the subscription has two_phase = on. I have to change this > option at catchup stage only, but this parameter can not be altered. There > was a patch proposal in past to implement altering of two_phase option, but > it was rejected. I think, the recreation of the subscription with two_phase > = off will not work. > > > The altering of two_phase was restricted because if there was a previously prepared transaction on the subscriber when the two_phase was on, and then it was turned off, the apply worker on the subscriber would re-apply the transaction a second time and this might result in an inconsistent replica. Here's a patch that allows toggling two_phase option provided that there are no pending uncommitted prepared transactions on the subscriber for that subscription. Thanks to Kuroda-san for working on the patch. regards, Ajin Cherian Fujitsu Australia Attachments: [application/octet-stream] v1-0001-Allow-altering-of-two_phase-option-in-subscribers.patch (7.7K, ../../CAFPTHDbU4EkVmKL66i5BzrmQ6LFftQPx_TZ1euKt9fvVzQDyjA@mail.gmail.com/3-v1-0001-Allow-altering-of-two_phase-option-in-subscribers.patch) download | inline diff: From bb19db9bd4e73d8892f202fb7b8771c25a033681 Mon Sep 17 00:00:00 2001 From: Ajin Cherian <[email protected]> Date: Thu, 4 Apr 2024 01:17:29 -0400 Subject: [PATCH v1] Allow altering of two_phase option in subscribers This patch allows user to alter two_phase option on a subscriber provided no uncommitted prepared transactions are pending on that subscription. --- src/backend/access/transam/twophase.c | 42 ++++++++++++++++++++++++++++++ src/backend/commands/subscriptioncmds.c | 35 ++++++++++++++++++++++--- src/backend/replication/logical/launcher.c | 22 +++++++++++++--- src/backend/replication/logical/worker.c | 3 --- src/include/access/twophase.h | 3 +++ src/include/replication/logicallauncher.h | 2 +- 6 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 8090ac9..b0aae25 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -2682,3 +2682,45 @@ LookupGXact(const char *gid, XLogRecPtr prepare_end_lsn, LWLockRelease(TwoPhaseStateLock); return found; } + +/* + * checkGid + */ +static bool +checkGid(char *gid, Oid subid) +{ + int ret; + Oid subid_written, xid; + + ret = sscanf(gid, "pg_gid_%u_%u", &subid_written, &xid); + + if (ret != 2 || subid != subid_written) + return false; + + return true; +} + +/* + * LookupGXactBySubid + * Check if the prepared transaction done by apply worker exists. + */ +bool +LookupGXactBySubid(Oid subid) +{ + bool found = false; + + LWLockAcquire(TwoPhaseStateLock, LW_SHARED); + for (int i = 0; i < TwoPhaseState->numPrepXacts; i++) + { + GlobalTransaction gxact = TwoPhaseState->prepXacts[i]; + + /* Ignore not-yet-valid GIDs. */ + if (gxact->valid && checkGid(gxact->gid, subid)) + { + found = true; + break; + } + } + LWLockRelease(TwoPhaseStateLock); + return found; +} diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 5a47fa9..8306929 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -16,6 +16,7 @@ #include "access/htup_details.h" #include "access/table.h" +#include "access/twophase.h" #include "access/xact.h" #include "catalog/catalog.h" #include "catalog/dependency.h" @@ -868,7 +869,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, pgstat_create_subscription(subid); if (opts.enabled) - ApplyLauncherWakeupAtCommit(); + ApplyLauncherWakeupAtEOXact(true); ObjectAddressSet(myself, SubscriptionRelationId, subid); @@ -1165,7 +1166,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, { supported_opts = (SUBOPT_SLOT_NAME | SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY | - SUBOPT_STREAMING | SUBOPT_DISABLE_ON_ERR | + SUBOPT_STREAMING | SUBOPT_TWOPHASE_COMMIT | + SUBOPT_DISABLE_ON_ERR | SUBOPT_PASSWORD_REQUIRED | SUBOPT_RUN_AS_OWNER | SUBOPT_FAILOVER | SUBOPT_ORIGIN); @@ -1173,6 +1175,31 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, parse_subscription_options(pstate, stmt->options, supported_opts, &opts); + /* XXX */ + if (IsSet(opts.specified_opts, SUBOPT_TWOPHASE_COMMIT)) + { + /* Stop corresponding worker */ + logicalrep_worker_stop(subid, InvalidOid); + + /* Request to start worker at the end of transaction */ + ApplyLauncherWakeupAtEOXact(false); + + /* Check whether the number of prepared transactions */ + if (!opts.twophase && + form->subtwophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED && + LookupGXactBySubid(subid)) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot disable two_phase when uncommitted prepared transactions present"))); + + /* Change system catalog acoordingly */ + values[Anum_pg_subscription_subtwophasestate - 1] = + CharGetDatum(opts.twophase ? + LOGICALREP_TWOPHASE_STATE_PENDING : + LOGICALREP_TWOPHASE_STATE_DISABLED); + replaces[Anum_pg_subscription_subtwophasestate - 1] = true; + } + if (IsSet(opts.specified_opts, SUBOPT_SLOT_NAME)) { /* @@ -1299,7 +1326,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, replaces[Anum_pg_subscription_subenabled - 1] = true; if (opts.enabled) - ApplyLauncherWakeupAtCommit(); + ApplyLauncherWakeupAtEOXact(true); update_tuple = true; break; @@ -1962,7 +1989,7 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) form->oid, 0); /* Wake up related background processes to handle this change quickly. */ - ApplyLauncherWakeupAtCommit(); + ApplyLauncherWakeupAtEOXact(true); LogicalRepWorkersWakeupAtCommit(form->oid); } diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c index 66070e9..899ec22 100644 --- a/src/backend/replication/logical/launcher.c +++ b/src/backend/replication/logical/launcher.c @@ -89,6 +89,7 @@ static dsa_area *last_start_times_dsa = NULL; static dshash_table *last_start_times = NULL; static bool on_commit_launcher_wakeup = false; +static bool launcher_wakeup = false; static void ApplyLauncherWakeup(void); @@ -1085,13 +1086,22 @@ ApplyLauncherForgetWorkerStartTime(Oid subid) void AtEOXact_ApplyLauncher(bool isCommit) { + bool kicked = false; + if (isCommit) { if (on_commit_launcher_wakeup) + { ApplyLauncherWakeup(); + kicked = true; + } } + if (!kicked && launcher_wakeup) + ApplyLauncherWakeup(); + on_commit_launcher_wakeup = false; + launcher_wakeup = false; } /* @@ -1102,10 +1112,16 @@ AtEOXact_ApplyLauncher(bool isCommit) * tuple was added to the pg_subscription catalog. */ void -ApplyLauncherWakeupAtCommit(void) +ApplyLauncherWakeupAtEOXact(bool on_commit) { - if (!on_commit_launcher_wakeup) - on_commit_launcher_wakeup = true; + if (on_commit) + { + if (!on_commit_launcher_wakeup) + on_commit_launcher_wakeup = true; + } + else + if (!launcher_wakeup) + launcher_wakeup = true; } static void diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index b5a80fe..ca3d260 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -3911,9 +3911,6 @@ maybe_reread_subscription(void) /* !slotname should never happen when enabled is true. */ Assert(newsub->slotname); - /* two-phase should not be altered */ - Assert(newsub->twophasestate == MySubscription->twophasestate); - /* * Exit if any parameter that affects the remote connection was changed. * The launcher will start a new worker but note that the parallel apply diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h index 56248c0..d493ed2 100644 --- a/src/include/access/twophase.h +++ b/src/include/access/twophase.h @@ -62,4 +62,7 @@ extern void PrepareRedoRemove(TransactionId xid, bool giveWarning); extern void restoreTwoPhaseData(void); extern bool LookupGXact(const char *gid, XLogRecPtr prepare_end_lsn, TimestampTz origin_prepare_timestamp); + +extern bool LookupGXactBySubid(Oid subid); + #endif /* TWOPHASE_H */ diff --git a/src/include/replication/logicallauncher.h b/src/include/replication/logicallauncher.h index ff0438b..075842c 100644 --- a/src/include/replication/logicallauncher.h +++ b/src/include/replication/logicallauncher.h @@ -24,7 +24,7 @@ extern void ApplyLauncherShmemInit(void); extern void ApplyLauncherForgetWorkerStartTime(Oid subid); -extern void ApplyLauncherWakeupAtCommit(void); +extern void ApplyLauncherWakeupAtEOXact(bool on_commit); extern void AtEOXact_ApplyLauncher(bool isCommit); extern bool IsLogicalLauncher(void); -- 1.8.3.1 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Slow catchup of 2PC (twophase) transactions on replica in LR @ 2024-04-04 05:37 Amit Kapila <[email protected]> parent: Ajin Cherian <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Amit Kapila @ 2024-04-04 05:37 UTC (permalink / raw) To: Ajin Cherian <[email protected]>; +Cc: Давыдов Виталий <[email protected]>; Heikki Linnakangas <[email protected]>; [email protected] On Thu, Apr 4, 2024 at 10:53 AM Ajin Cherian <[email protected]> wrote: > > On Wed, Mar 6, 2024 at 1:29 AM Давыдов Виталий <[email protected]> wrote: >> >> In usual work, the subscription has two_phase = on. I have to change this option at catchup stage only, but this parameter can not be altered. There was a patch proposal in past to implement altering of two_phase option, but it was rejected. I think, the recreation of the subscription with two_phase = off will not work. >> >> > > The altering of two_phase was restricted because if there was a previously prepared transaction on the subscriber when the two_phase was on, and then it was turned off, the apply worker on the subscriber would re-apply the transaction a second time and this might result in an inconsistent replica. > Here's a patch that allows toggling two_phase option provided that there are no pending uncommitted prepared transactions on the subscriber for that subscription. > I think this would probably be better than the current situation but can we think of a solution to allow toggling the value of two_phase even when prepared transactions are present? Can you please summarize the reason for the problems in doing that and the solutions, if any? -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2024-04-04 05:37 UTC | newest] Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-11-06 00:58 [PATCH v7 4/7] Invalidate parent indexes Justin Pryzby <[email protected]> 2024-02-22 13:29 Slow catchup of 2PC (twophase) transactions on replica in LR Давыдов Виталий <[email protected]> 2024-02-23 03:23 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Amit Kapila <[email protected]> 2024-02-23 17:11 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Давыдов Виталий <[email protected]> 2024-02-26 13:24 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Amit Kapila <[email protected]> 2024-02-27 11:19 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Давыдов Виталий <[email protected]> 2024-02-27 13:00 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Amit Kapila <[email protected]> 2024-02-27 13:34 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Давыдов Виталий <[email protected]> 2024-02-29 17:34 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Давыдов Виталий <[email protected]> 2024-03-05 09:05 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Heikki Linnakangas <[email protected]> 2024-03-05 14:29 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Давыдов Виталий <[email protected]> 2024-03-06 12:25 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Amit Kapila <[email protected]> 2024-04-04 05:23 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Ajin Cherian <[email protected]> 2024-04-04 05:37 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Amit Kapila <[email protected]> 2024-02-23 04:52 ` Re: Slow catchup of 2PC (twophase) transactions on replica in LR Ajin Cherian <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox