From 88a16b3dbb16dc127894ad56c57c53d309ae816a Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 27 Nov 2024 09:31:40 +0900 Subject: [PATCH] Handle better implicit transaction state of pipeline mode When using a pipeline, a transaction starts from the first command and is committed with a Sync message or when the pipeline ends. Functions like IsInTransactionBlock() or PreventInTransactionBlock() were already able to understand a pipeline as being in a transaction block, but it was not the case of CheckTransactionBlock(). This function is called for example to generate a WARNING for SET LOCAL, complaining that it is used outside of a transaction block. The current state of the code caused multiple problems, like: - SET LOCAL executed at any stage of a pipeline issued a WARNING, even if the command was at least second in line where the pipeline is in a transaction state. - LOCK TABLE failed when invoked at any step of a pipeline, even if it should be able to work within a transaction block. The pipeline protocol assumes that the first command of a pipeline is not part of a transaction block, and that any follow-up commands is considered as within a transaction block. This commit changes the backend so as an implicit transaction block is started each time the first Execute message of a pipeline has finished processing, with this implicit transaction block ended once a sync is processed. The checks based on XACT_FLAGS_PIPELINING in the routines checking if we are in a transaction block are not necessary: it is enough to rely on the existing ones. Some tests are added to pgbench, that can be backpatched down to v17 when \syncpipeline is involved and down to v14 where \startpipeline and \endpipeline are available. This is unfortunately limited regarding the error patterns that can be checked, but it provides coverage for various pipeline combinations to check if these succeed or fail. These tests are able to capture the case of SET LOCAL's WARNING. The author has proposed a different feature to improve the coverage by adding similar meta-commands to psql where error messages could be checked, something more useful for the cases where commands cannot be used in transaction blocks, like REINDEX CONCURRENTLY or VACUUM. This is considered as future work for v18~. Author: Anthonin Bonnefoy Reviewed-by: Jelte Fennema-Nio, Michael Paquier Discussion: https://postgr.es/m/CAO6_XqrWO8uNBQrSu5r6jh+vTGi5Oiyk4y8yXDORdE2jbzw8xw@mail.gmail.com Backpatch-through: 13 --- doc/src/sgml/protocol.sgml | 21 +-- src/backend/access/transam/xact.c | 13 -- src/backend/tcop/postgres.c | 18 +++ src/bin/pgbench/t/001_pgbench_with_server.pl | 155 +++++++++++++++++++ 4 files changed, 184 insertions(+), 23 deletions(-) diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index b6c07b1d053..a4e64383a96 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1088,16 +1088,17 @@ SELCT 1/0; If the client has not issued an explicit BEGIN, - then each Sync ordinarily causes an implicit COMMIT - if the preceding step(s) succeeded, or an - implicit ROLLBACK if they failed. However, there - are a few DDL commands (such as CREATE DATABASE) - that cannot be executed inside a transaction block. If one of - these is executed in a pipeline, it will fail unless it is the first - command in the pipeline. Furthermore, upon success it will force an - immediate commit to preserve database consistency. Thus a Sync - immediately following one of these commands has no effect except to - respond with ReadyForQuery. + then an implicit transaction block is started and each Sync ordinarily + causes an implicit COMMIT if the preceding step(s) + succeeded, or an implicit ROLLBACK if they failed. + This implicit transaction block will only be detected by the server + when the first command ends without a sync. There are a few DDL + commands (such as CREATE DATABASE) that cannot be + executed inside a transaction block. If one of these is executed in a + pipeline, it will fail unless it is the first command after a Sync. + Furthermore, upon success it will force an immediate commit to preserve + database consistency. Thus a Sync immediately following one of these + commands has no effect except to respond with ReadyForQuery. diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 7a3d9b4b012..20ebc24194f 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -3489,16 +3489,6 @@ PreventInTransactionBlock(bool isTopLevel, const char *stmtType) errmsg("%s cannot run inside a subtransaction", stmtType))); - /* - * inside a pipeline that has started an implicit transaction? - */ - if (MyXactFlags & XACT_FLAGS_PIPELINING) - ereport(ERROR, - (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION), - /* translator: %s represents an SQL statement name */ - errmsg("%s cannot be executed within a pipeline", - stmtType))); - /* * inside a function call? */ @@ -3610,9 +3600,6 @@ IsInTransactionBlock(bool isTopLevel) if (IsSubTransaction()) return true; - if (MyXactFlags & XACT_FLAGS_PIPELINING) - return true; - if (!isTopLevel) return true; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index c8c687d6f51..237c8ab00a2 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -2746,6 +2746,17 @@ start_xact_command(void) xact_started = true; } + else if (MyXactFlags & XACT_FLAGS_PIPELINING) + { + /* + * When the first Execute message is completed, following commands + * will be done in an implicit transaction block created via + * pipelining. The transaction state needs to be updated to an + * implicit block if we're not already in a transaction block (like + * one started by an explicit BEGIN). + */ + BeginImplicitTransactionBlock(); + } /* * Start statement timeout if necessary. Note that this'll intentionally @@ -4865,6 +4876,13 @@ PostgresMain(const char *dbname, const char *username) case 'S': /* sync */ pq_getmsgend(&input_message); + + /* + * If pipelining was used, we may be in an implicit + * transaction block. Close it before calling + * finish_xact_command. + */ + EndImplicitTransactionBlock(); finish_xact_command(); send_ready_for_query = true; break; diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl index f9690c64957..7915f6fd0aa 100644 --- a/src/bin/pgbench/t/001_pgbench_with_server.pl +++ b/src/bin/pgbench/t/001_pgbench_with_server.pl @@ -897,6 +897,161 @@ $node->pgbench( } }); +# Try SET LOCAL as first pipeline command. This succeeds and the first +# command is not executed inside an implicit transaction block, causing +# a WARNING. +$node->pgbench( + '-t 1 -n -M extended', + 0, + [], + [qr{WARNING: SET LOCAL can only be used in transaction blocks}], + 'SET LOCAL outside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_set_local_1' => q{ +\startpipeline +SET LOCAL statement_timeout='1h'; +\endpipeline +} + }); + +# Try SET LOCAL as second pipeline command. This succeeds and the second +# command does not cause a WARNING to be generated. +$node->pgbench( + '-t 1 -n -M extended', + 0, + [], + [qr{^$}], + 'SET LOCAL inside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_set_local_2' => q{ +\startpipeline +SELECT 1; +SET LOCAL statement_timeout='1h'; +\endpipeline +} + }); + +# Try REINDEX CONCURRENTLY as first pipeline command. This succeeds +# as the first command is outside the implicit transaction block of +# a pipeline. +$node->pgbench( + '-t 1 -n -M extended', + 0, + [], + [], + 'REINDEX CONCURRENTLY outside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_reindex_1' => q{ +\startpipeline +REINDEX TABLE CONCURRENTLY pgbench_accounts; +SELECT 1; +\endpipeline +} + }); + +# Try REINDEX CONCURRENTLY as second pipeline command. This fails +# as the second command is inside an implicit transaction block. +$node->pgbench( + '-t 1 -n -M extended', + 2, + [], + [], + 'error: REINDEX CONCURRENTLY inside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_reindex_2' => q{ +\startpipeline +SELECT 1; +REINDEX TABLE CONCURRENTLY pgbench_accounts; +\endpipeline +} + }); + +# Try VACUUM as first pipeline command. Like REINDEX CONCURRENTLY, this +# succeeds as this is outside the implicit transaction block of a pipeline. +$node->pgbench( + '-t 1 -n -M extended', + 0, + [], + [], + 'VACUUM outside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_vacuum_1' => q{ +\startpipeline +VACUUM pgbench_accounts; +\endpipeline +} + }); + +# Try VACUUM as second pipeline command. This fails, as the second command +# of a pipeline is inside an implicit transaction block. +$node->pgbench( + '-t 1 -n -M extended', + 2, + [], + [], + 'error: VACUUM inside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_vacuum_2' => q{ +\startpipeline +SELECT 1; +VACUUM pgbench_accounts; +\endpipeline +} + }); + +# Try subtransactions in a pipeline. These are forbidden in implicit +# transaction blocks. +$node->pgbench( + '-t 1 -n -M extended', + 2, + [], + [], + 'error: subtransactions not allowed in pipeline', + { + '001_pgbench_pipeline_subtrans' => q{ +\startpipeline +SAVEPOINT a; +SELECT 1; +ROLLBACK TO SAVEPOINT a; +SELECT 2; +\endpipeline +} + }); + +# Try LOCK TABLE as first pipeline command. This fails as LOCK is outside +# an implicit transaction block. +$node->pgbench( + '-t 1 -n -M extended', + 2, + [], + [], + 'error: LOCK TABLE outside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_lock_1' => q{ +\startpipeline +LOCK pgbench_accounts; +SELECT 1; +\endpipeline +} + }); + +# Try LOCK TABLE as second pipeline command. This succeeds as LOCK is inside +# an implicit transaction block. +$node->pgbench( + '-t 1 -n -M extended', + 0, + [], + [], + 'LOCK TABLE inside implicit transaction block of pipeline', + { + '001_pgbench_pipeline_lock_2' => q{ +\startpipeline +SELECT 1; +LOCK pgbench_accounts; +\endpipeline +} + }); + # Working \startpipeline in prepared query mode with serializable $node->pgbench( '-c4 -t 10 -n -M prepared',