From ceef6341518532fa175e8d764a875d7e854a83ac Mon Sep 17 00:00:00 2001 From: Konstantin Knizhnik Date: Sat, 25 Jan 2025 14:23:17 +0200 Subject: [PATCH 1/4] Add support of dedicated backends --- src/backend/access/transam/xact.c | 6 +++++- src/backend/commands/portalcmds.c | 3 +++ src/backend/commands/sequence.c | 15 +++++++++++++++ src/backend/commands/tablecmds.c | 4 ++++ src/backend/storage/lmgr/lock.c | 3 +++ src/backend/utils/adt/lockfuncs.c | 3 +++ src/backend/utils/misc/guc_funcs.c | 3 +++ src/backend/utils/misc/guc_tables.c | 12 ++++++++++++ src/include/access/xact.h | 3 +++ 9 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 4cecf630060..3a07ea420d3 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -84,6 +84,10 @@ bool XactDeferrable; int synchronous_commit = SYNCHRONOUS_COMMIT_ON; +/* Backend needs to preserve session context */ +bool is_dedicated_backend; +bool allow_dedicated_backends; /* GUC */ + /* * CheckXidAlive is a xid value pointing to a possibly ongoing (sub) * transaction. Currently, it is used in logical decoding. It's possible @@ -4952,7 +4956,7 @@ TransactionBlockStatusCode(void) { case TBLOCK_DEFAULT: case TBLOCK_STARTED: - return 'I'; /* idle --- not in transaction */ + return is_dedicated_backend && allow_dedicated_backends ? 'T' : 'I'; /* idle --- not in transaction */ case TBLOCK_BEGIN: case TBLOCK_SUBBEGIN: case TBLOCK_INPROGRESS: diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 4f6acf67198..390896aa286 100644 --- a/src/backend/commands/portalcmds.c +++ b/src/backend/commands/portalcmds.c @@ -59,6 +59,9 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa (errcode(ERRCODE_INVALID_CURSOR_NAME), errmsg("invalid cursor name: must not be empty"))); + if (cstmt->options & CURSOR_OPT_HOLD) + is_dedicated_backend = true; /* cursors are not compatible with builtin connection pooler */ + /* * If this is a non-holdable cursor, we require that this statement has * been executed inside a transaction block (or else, it would have no diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index a29550ccad8..edca49bdfbd 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -245,6 +245,19 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq) heap_freetuple(tuple); table_close(rel, RowExclusiveLock); + /* + * TODO: + * Using currval() may cause incorrect behavior with connection pooler. + * Unfortunately marking backend as tainted in currval() is too late. + * This is why it is done in nextval(), although it is not strictly required, because + * nextval() may be not followed by currval(). + * But currval() may be not preceded by nextval(). + * To make regression tests passed, backend is also marker as tainted when it creates + * sequence. Certainly it is just temporary workaround, because sequence may be created + * in one backend and accessed in another. + */ + is_dedicated_backend = true; /* in case of using currval() */ + return address; } @@ -611,6 +624,8 @@ nextval(PG_FUNCTION_ARGS) */ relid = RangeVarGetRelid(sequence, NoLock, false); + is_dedicated_backend = true; /* in case of using currval() */ + PG_RETURN_INT64(nextval_internal(relid, true)); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 36717ffcb0a..61db1b49f7b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -733,6 +733,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("ON COMMIT can only be used on temporary tables"))); + if (stmt->relation->relpersistence == RELPERSISTENCE_TEMP + && stmt->oncommit != ONCOMMIT_DROP) + is_dedicated_backend = true; + if (stmt->partspec != NULL) { if (relkind != RELKIND_RELATION) diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index e5e7ab55716..26aec299dfd 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -822,7 +822,10 @@ LockAcquireExtended(const LOCKTAG *locktag, /* Identify owner for lock */ if (sessionLock) + { owner = NULL; + is_dedicated_backend = true; + } else owner = CurrentResourceOwner; diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c index e790f856ab3..d98e60fee3f 100644 --- a/src/backend/utils/adt/lockfuncs.c +++ b/src/backend/utils/adt/lockfuncs.c @@ -13,6 +13,7 @@ #include "postgres.h" #include "access/htup_details.h" +#include "access/xact.h" #include "funcapi.h" #include "miscadmin.h" #include "storage/predicate_internals.h" @@ -611,12 +612,14 @@ pg_safe_snapshot_blocking_pids(PG_FUNCTION_ARGS) * field4: 1 if using an int8 key, 2 if using 2 int4 keys */ #define SET_LOCKTAG_INT64(tag, key64) \ + is_dedicated_backend = true; \ SET_LOCKTAG_ADVISORY(tag, \ MyDatabaseId, \ (uint32) ((key64) >> 32), \ (uint32) (key64), \ 1) #define SET_LOCKTAG_INT32(tag, key1, key2) \ + is_dedicated_backend = true; \ SET_LOCKTAG_ADVISORY(tag, MyDatabaseId, key1, key2, 2) /* diff --git a/src/backend/utils/misc/guc_funcs.c b/src/backend/utils/misc/guc_funcs.c index 9c9edd3d2f5..b1d80ea1b15 100644 --- a/src/backend/utils/misc/guc_funcs.c +++ b/src/backend/utils/misc/guc_funcs.c @@ -53,6 +53,9 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) (errcode(ERRCODE_INVALID_TRANSACTION_STATE), errmsg("cannot set parameters during a parallel operation"))); + if (!stmt->is_local) + is_dedicated_backend = true; + switch (stmt->kind) { case VAR_SET_VALUE: diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 69c8238de49..fc80f323095 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -802,6 +802,18 @@ struct config_bool ConfigureNamesBool[] = true, NULL, NULL, NULL }, + + { + {"allow_dedicated_backends", PGC_USERSET, CLIENT_CONN_OTHER, + gettext_noop("Prevent connection pooler from reusing backends requiring session semantic."), + NULL, + GUC_EXPLAIN + }, + &allow_dedicated_backends, + true, + NULL, NULL, NULL + }, + { {"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD, gettext_noop("Enables the planner's use of sequential-scan plans."), diff --git a/src/include/access/xact.h b/src/include/access/xact.h index 6d4439f0524..3c199ac31ca 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -58,6 +58,9 @@ extern PGDLLIMPORT bool XactReadOnly; /* flag for logging statements in this transaction */ extern PGDLLIMPORT bool xact_is_sampled; +/* Backend needs to preserve session context */ +extern PGDLLIMPORT bool is_dedicated_backend; +extern bool allow_dedicated_backends; /* * Xact is deferrable -- only meaningful (currently) for read only * SERIALIZABLE transactions From 509559d90c711da3e298da081ef60a3816468f5e Mon Sep 17 00:00:00 2001 From: Konstantin Knizhnik Date: Sat, 25 Jan 2025 20:49:25 +0200 Subject: [PATCH 2/4] Disable dedicated backend for psql --- src/backend/utils/init/postinit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index adc21e019d5..3201b1f9717 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -1268,6 +1268,9 @@ InitPostgres(const char *in_dbname, Oid dboid, /* close the transaction we started above */ if (!bootstrap) CommitTransactionCommand(); + + if (strcmp(application_name, "psql") == 0) + allow_dedicated_backends = false; } /* From 4a7b8c941bf7b7726d2d05cd9df04837a6ffade3 Mon Sep 17 00:00:00 2001 From: Konstantin Knizhnik Date: Sun, 26 Jan 2025 08:51:05 +0200 Subject: [PATCH 3/4] Disable dedicated backends for regression tests --- src/backend/utils/init/postinit.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 3201b1f9717..ff1def4569b 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -1269,7 +1269,15 @@ InitPostgres(const char *in_dbname, Oid dboid, if (!bootstrap) CommitTransactionCommand(); - if (strcmp(application_name, "psql") == 0) + /* + * TODO: psql logic and prompt depends on status returned by ReadyForQuery message. + * So the hack with enforcing dedicated backend by reporting in-transaction status doesn't + * work in this case. + * The solution can be to return some special status in ReadyForQuery message which + * will be interpreted only by connection pooler but then mapped to idle ('I') when + * forwarded to the client. + */ + if (strcmp(application_name, "psql") == 0 || strcmp(application_name, "pg_regress") == 0) allow_dedicated_backends = false; } From 6fda5f18296e1c5c81d0ed5574bd30d1e1ea2fb6 Mon Sep 17 00:00:00 2001 From: Konstantin Knizhnik Date: Sun, 26 Jan 2025 22:44:19 +0200 Subject: [PATCH 4/4] Disable dedicated backend for psql --- src/backend/utils/init/postinit.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index ff1def4569b..3aedb92212a 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -1277,8 +1277,11 @@ InitPostgres(const char *in_dbname, Oid dboid, * will be interpreted only by connection pooler but then mapped to idle ('I') when * forwarded to the client. */ - if (strcmp(application_name, "psql") == 0 || strcmp(application_name, "pg_regress") == 0) - allow_dedicated_backends = false; + if (application_name) + { + if (strcmp(application_name, "psql") == 0 || strncmp(application_name, "pg_regress", 10) == 0) + allow_dedicated_backends = false; + } } /*