Re: Synchronizing slots from primary to standby
Hi all, Peter Eisentraut writes: > I want to reactivate $subject. I took Petr Jelinek's patch from [0], > rebased it, added a bit of testing. It basically works, but as > mentioned in [0], there are various issues to work out. Thanks for working on that topic, I believe it's an important part of Postgres' HA story. > The idea is that the standby runs a background worker to periodically fetch > replication slot information from the primary. On failover, a logical > subscriber would then ideally find up-to-date replication slots on the new > publisher and can just continue normally. Is there a case to be made about doing the same thing for physical replication slots too? That's what pg_auto_failover [1] does by default: it creates replication slots on every node for every other node, in a way that a standby Postgres instance now maintains a replication slot for the primary. This ensures that after a promotion, the standby knows to retain any and all WAL segments that the primary might need when rejoining, at pg_rewind time. > The previous thread didn't have a lot of discussion, but I have gathered > from off-line conversations that there is a wider agreement on this > approach. So the next steps would be to make it more robust and > configurable and documented. I suppose part of the configuration would then include taking care of physical slots. Some people might want to turn that off and use the Postgres 13+ ability to use the remote primary restore_command to fetch missing WAL files, instead. Well, people who have setup an archiving system, anyway. > As I said, I added a small test case to > show that it works at all, but I think a lot more tests should be added. I > have also found that this breaks some seemingly unrelated tests in the > recovery test suite. I have disabled these here. I'm not sure if the patch > actually breaks anything or if these are just differences in timing or > implementation dependencies. This patch adds a LIST_SLOTS replication > command, but I think this could be replaced with just a SELECT FROM > pg_replication_slots query now. (This patch is originally older than when > you could run SELECT queries over the replication protocol.) Given the admitted state of the patch, I didn't focus on tests. I could successfully apply the patch on-top of current master's branch, and cleanly compile and `make check`. Then I also updated pg_auto_failover to support Postgres 15devel [2] so that I could then `make NODES=3 cluster` there and play with the new replication command: $ psql -d "port=5501 replication=1" -c "LIST_SLOTS;" psql:/Users/dim/.psqlrc:24: ERROR: XX000: cannot execute SQL commands in WAL sender for physical replication LOCATION: exec_replication_command, walsender.c:1830 ... I'm not too sure about this idea of running SQL in a replication protocol connection that you're mentioning, but I suppose that's just me needing to brush up on the topic. > So, again, this isn't anywhere near ready, but there is already a lot here > to gather feedback about how it works, how it should work, how to configure > it, and how it fits into an overall replication and HA architecture. Maybe the first question about configuration would be about selecting which slots a standby should maintain from the primary. Is it all of the slots that exists on both the nodes, or a sublist of that? Is it possible to have a slot with the same name on a primary and a standby node, in a way that the standby's slot would be a completely separate entity from the primary's slot? If yes (I just don't know at the moment), well then, should we continue to allow that? Other aspects of the configuration might include a list of databases in which to make the new background worker active, and the polling delay, etc. Also, do we want to even consider having the slot management on a primary node depend on the ability to sync the advancing on one or more standby nodes? I'm not sure to see that one as a good idea, but maybe we want to kill it publically very early then ;-) Regards, -- dim Author of “The Art of PostgreSQL”, see https://theartofpostgresql.com [1]: https://github.com/citusdata/pg_auto_failover [2]: https://github.com/citusdata/pg_auto_failover/pull/838
Re: Built-in connection pooler
Hi, Bruce Momjian writes: > It is nice it is a smaller patch. Please remind me of the performance > advantages of this patch. The patch as it stands is mostly helpful in those situations: - application server(s) start e.g. 2000 connections at start-up and then use them depending on user traffic It's then easy to see that if we would only fork as many backends as we need, while having accepted the 2000 connections without doing anything about them, we would be in a much better position than when we fork 2000 unused backends. - application is partially compatible with pgbouncer transaction pooling mode Then in that case, you would need to run with pgbouncer in session mode. This happens when the application code is using session level SQL commands/objects, such as prepared statements, temporary tables, or session-level GUCs settings. With the attached patch, if the application sessions profiles are mixed, then you dynamically get the benefits of transaction pooling mode for those sessions which are not “tainting” the backend, and session pooling mode for the others. It means that it's then possible to find the most often used session and fix that one for immediate benefits, leaving the rest of the code alone. If it turns out that 80% of your application sessions are the same code-path and you can make this one “transaction pooling” compatible, then you most probably are fixing (up to) 80% of your connection-related problems in production. - applications that use a very high number of concurrent sessions In that case, you can either set your connection pooling the same as max_connection and see no benefits (and hopefully no regressions either), or set a lower number of backends serving a very high number of connections, and have sessions waiting their turn at the “proxy” stage. This is a kind of naive Admission Control implementation where it's better to have active clients in the system wait in line consuming as few resources as possible. Here, in the proxy. It could be done with pgbouncer already, this patch gives a stop-gap in PostgreSQL itself for those use-cases. It would be mostly useful to do that when you have queries that are benefiting of parallel workers. In that case, controling the number of active backend forked at any time to serve user queries allows to have better use of the parallel workers available. In other cases, it's important to measure and accept the possible performance cost of running a proxy server between the client connection and the PostgreSQL backend process. I believe the numbers shown in the previous email by Konstantin are about showing the kind of impact you can see when using the patch in a use-case where it's not meant to be helping much, if at all. Regards, -- dim
Prepare Transaction support for ON COMMIT DROP temporary tables
Hi,
Please find attached a patch to enable support for temporary tables in
prepared transactions when ON COMMIT DROP has been specified.
The comment in the existing code around this idea reads:
* Don't allow PREPARE TRANSACTION if we've accessed a temporary table
in
* this transaction.
[ ... ]
* XXX In principle this could be relaxed to allow some useful special
* cases, such as a temp table created and dropped all within the
* transaction. That seems to require much more bookkeeping though.
In the attached patch I have added this paragraph, and of course the
implementation of it:
* A special case of this situation is using ON COMMIT DROP, where the
* call to PreCommit_on_commit_actions() is then responsible for
* performing the DROP table within the transaction and before we get
* here.
Regards,
--
dim
commit 7dd834a2fb57ba617d70abf7a23eb5cc84dadca5
Author: Dimitri Fontaine
Date: Thu Dec 27 12:22:56 2018 +0100
Add support for on-commit-drop temp tables to prepared transactions.
As the bookkeeping is all done within PreCommit_on_commit_actions() we can
prepare a transaction that have been accessing temporary tables when all of
them as marked ON COMMIT DROP.
diff --git a/doc/src/sgml/ref/prepare_transaction.sgml b/doc/src/sgml/ref/prepare_transaction.sgml
index d958f7a06f..c1ef0707cf 100644
--- a/doc/src/sgml/ref/prepare_transaction.sgml
+++ b/doc/src/sgml/ref/prepare_transaction.sgml
@@ -97,8 +97,9 @@ PREPARE TRANSACTION transaction_id
- It is not currently allowed to PREPARE a transaction that
- has executed any operations involving temporary tables,
+ It is not currently allowed to PREPARE a transaction
+ that has executed any operations involving temporary tables (except when
+ all involved temporary tables are ON COMMIT DROP),
created any cursors WITH HOLD, or executed
LISTEN, UNLISTEN, or
NOTIFY.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d967400384..1d7f3017ad 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2272,11 +2272,19 @@ PrepareTransaction(void)
* XXX In principle this could be relaxed to allow some useful special
* cases, such as a temp table created and dropped all within the
* transaction. That seems to require much more bookkeeping though.
+ *
+ * A special case of this situation is using ON COMMIT DROP, where the
+ * call to PreCommit_on_commit_actions() is then responsible for
+ * performing the DROP table within the transaction and before we get
+ * here.
*/
- if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL))
+ if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL)
+ && !every_on_commit_is_on_commit_drop())
+ {
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
+ errmsg("cannot PREPARE a transaction that has operated on temporary tables that are not ON COMMIT DROP")));
+ }
/*
* Likewise, don't allow PREPARE after pg_export_snapshot. This could be
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c8c50e8c98..2060f3e68e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13139,6 +13139,26 @@ remove_on_commit_action(Oid relid)
}
}
+/*
+ * Return true when every temp relation known in the current session is marked
+ * ON COMMIT DROP. This allows to then PREPARE TRANSACTION, for instance,
+ * because we know that at prepare time the temp table is dropped already.
+ */
+bool
+every_on_commit_is_on_commit_drop()
+{
+ ListCell *l;
+
+ foreach(l, on_commits)
+ {
+ OnCommitItem *oc = (OnCommitItem *) lfirst(l);
+
+ if (oc->oncommit != ONCOMMIT_DROP)
+ return false;
+ }
+ return true;
+}
+
/*
* Perform ON COMMIT actions.
*
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 2afcd5be44..2f5731a9f6 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -82,6 +82,7 @@ extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
extern void register_on_commit_action(Oid relid, OnCommitAction action);
extern void remove_on_commit_action(Oid relid);
+extern bool every_on_commit_is_on_commit_drop(void);
extern void PreCommit_on_commit_actions(void);
extern void AtEOXact_on_commit_actions(bool isCommit);
diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out
index eb77c18788..a50ca764c5 100644
--- a/src/test/regress/expected/prepared_xacts.out
+++ b/src/test/regress/expected/prepared_xacts.out
@@ -252,3 +252,95 @@ DROP TABLE pxtest2;
DROP TABLE pxtest3; -- will still be there if prepared xacts are disabled
ERROR: table "pxtest3" does not exist
DROP TABLE pxtest
Re: Prepare Transaction support for ON COMMIT DROP temporary tables
Alvaro Herrera writes:
> Glad to see you submitting patches again.
Thanks!
> I suggest to add in your regression tests a case where the prepared
> transaction commits, and ensuring that the temp table is gone from
> catalogs.
Please find such a revision attached.
Regards,
--
dim
commit 0b2b2d4b2ec4cd6cd2d26ef229a3764ab9ad2e78
Author: Dimitri Fontaine
Date: Thu Dec 27 12:22:56 2018 +0100
Add support for on-commit-drop temp tables to prepared transactions.
As the bookkeeping is all done within PreCommit_on_commit_actions() we can
prepare a transaction that have been accessing temporary tables when all of
them as marked ON COMMIT DROP.
diff --git a/doc/src/sgml/ref/prepare_transaction.sgml b/doc/src/sgml/ref/prepare_transaction.sgml
index d958f7a06f..c1ef0707cf 100644
--- a/doc/src/sgml/ref/prepare_transaction.sgml
+++ b/doc/src/sgml/ref/prepare_transaction.sgml
@@ -97,8 +97,9 @@ PREPARE TRANSACTION transaction_id
- It is not currently allowed to PREPARE a transaction that
- has executed any operations involving temporary tables,
+ It is not currently allowed to PREPARE a transaction
+ that has executed any operations involving temporary tables (except when
+ all involved temporary tables are ON COMMIT DROP),
created any cursors WITH HOLD, or executed
LISTEN, UNLISTEN, or
NOTIFY.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d967400384..1d7f3017ad 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2272,11 +2272,19 @@ PrepareTransaction(void)
* XXX In principle this could be relaxed to allow some useful special
* cases, such as a temp table created and dropped all within the
* transaction. That seems to require much more bookkeeping though.
+ *
+ * A special case of this situation is using ON COMMIT DROP, where the
+ * call to PreCommit_on_commit_actions() is then responsible for
+ * performing the DROP table within the transaction and before we get
+ * here.
*/
- if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL))
+ if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL)
+ && !every_on_commit_is_on_commit_drop())
+ {
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
+ errmsg("cannot PREPARE a transaction that has operated on temporary tables that are not ON COMMIT DROP")));
+ }
/*
* Likewise, don't allow PREPARE after pg_export_snapshot. This could be
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c8c50e8c98..2060f3e68e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13139,6 +13139,26 @@ remove_on_commit_action(Oid relid)
}
}
+/*
+ * Return true when every temp relation known in the current session is marked
+ * ON COMMIT DROP. This allows to then PREPARE TRANSACTION, for instance,
+ * because we know that at prepare time the temp table is dropped already.
+ */
+bool
+every_on_commit_is_on_commit_drop()
+{
+ ListCell *l;
+
+ foreach(l, on_commits)
+ {
+ OnCommitItem *oc = (OnCommitItem *) lfirst(l);
+
+ if (oc->oncommit != ONCOMMIT_DROP)
+ return false;
+ }
+ return true;
+}
+
/*
* Perform ON COMMIT actions.
*
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 2afcd5be44..2f5731a9f6 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -82,6 +82,7 @@ extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
extern void register_on_commit_action(Oid relid, OnCommitAction action);
extern void remove_on_commit_action(Oid relid);
+extern bool every_on_commit_is_on_commit_drop(void);
extern void PreCommit_on_commit_actions(void);
extern void AtEOXact_on_commit_actions(bool isCommit);
diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out
index eb77c18788..d2e81461d0 100644
--- a/src/test/regress/expected/prepared_xacts.out
+++ b/src/test/regress/expected/prepared_xacts.out
@@ -252,3 +252,101 @@ DROP TABLE pxtest2;
DROP TABLE pxtest3; -- will still be there if prepared xacts are disabled
ERROR: table "pxtest3" does not exist
DROP TABLE pxtest4;
+-- we should be able to prepare a transaction with on commit drop temporary
+-- tables
+begin;
+create temp table pxtesttemp(id serial, f1 text) on commit drop;
+insert into pxtesttemp(f1) values('regress'), ('temp') returning *;
+ id | f1
++-
+ 1 | regress
+ 2 | temp
+(2 rows)
+
+prepare transaction 'regress temp';
+commit prepared 'regress temp';
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+ gid
+-
+(0 rows)
+
+-- The temporary table should be gone now
+SELECT relname FROM pg_class WHERE relname = 'pxte
Re: Prepare Transaction support for ON COMMIT DROP temporary tables
Hi, Tom Lane writes: > I'm afraid that this patch is likely to be, if not completely broken, > at least very much less useful than one could wish by the time we get > done closing the holes discussed in this other thread: > > https://www.postgresql.org/message-id/flat/5d910e2e-0db8-ec06-dd5f-baec420513c3%40imap.cc Thanks for the review here Tom, and for linking with the other discussion (Álvaro did that too, thanks!). I've been reviewing it too. I didn't think about the pg_temp_NN namespaces in my approach, and I think it might be possible to make it work, but it's getting quite involved now. One idea would be that if every temp table in the session belongs to the transaction, and their namespace too (we could check through pg_depend that the namespace doesn't contain anything else beside the transaction's tables), then we could dispose of the temp schema and on-commit-drop tables at PREPARE COMMIT time. Otherwise, as before, prevent the transaction to be a 2PC one. > For instance, if we're going to have to reject the case where the > session's temporary schema was created during the current transaction, > then that puts a very weird constraint on whether this case works. Yeah. The goal of my approach is to transparently get back temp table support in 2PC when that makes sense, which is most use cases I've been confronted to. We use 2PC in Citus, and it would be nice to be able to use transaction local temp tables on worker nodes when doing data injestion and roll-ups. > Also, even without worrying about new problems that that discussion > may lead to, I don't think that the patch works as-is. The function > every_on_commit_is_on_commit_drop() does what it says, but that is > NOT sufficient to conclude that every temp table the transaction has > touched is on-commit-drop. This logic will successfully reject cases > with on-commit-delete-rows temp tables, but not cases where the temp > table(s) lack any ON COMMIT spec at all. Thanks! I missed that the lack of ON COMMIT spec would have that impact in the code. We could add tracking of that I suppose, and will have a look at how to implement it provided that the other points find an acceptable solution. Regards, -- dim
