pgpool2.git / summary / log / commit / refs
commit eea522ebfcf791a623e865deaa1aa6fb59e3c50b
Author: Tatsuo Ishii <ishii@sraoss.co.jp>
Date: Sun May 08 09:00:20 2022 +0000
Fix not to abort session while in failed transaction.
When an explicit transaction fails, subsequent commands should be
ignored until commit or rollback command comes in. However pgpool
sometimes aborted the session if relcache lookup happened. example:
test=# begin;
BEGIN
test=*# insert into ttt values(1);
ERROR: relation "ttt" does not exist
LINE 1: insert into ttt values(1);
^
test=!# select * from t1;
FATAL: Backend throw an error message
DETAIL: Exiting current session because of an error from backend
HINT: BACKEND Error: "current transaction is aborted, commands ignored until end of transaction block"
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
To prevent this, check the transaction state and if it's already in
aborted state, do not try to process the command, but just reply back
to frontend with an error message:
ERROR: current transaction is aborted, commands ignored until end of transaction block
By the way, while implementing the fix, I found old bugs with replication mode.
- ReadyForQuery() mistakenly called end_internal_transaction() even if
it's not in replication/si mode. Moreover,
end_internal_transaction() is called even if the internal
transaction has not started.
- end_internal_transaction() did not call pool_unset_failed_transaction();
These bugs were harmless until I tried to fix the issue (session
aborting).
New regression test 078 for this added.
Discussion: https://www.pgpool.net/pipermail/pgpool-general/2022-April/008155.html
src/protocol/pool_process_query.c | 1 +
src/protocol/pool_proto_modules.c | 70 +++++++++++++++++++++-
.../tests/078.aborted_transaction/expected.txt | 28 +++++++++
.../tests/078.aborted_transaction/test.sh | 48 +++++++++++++++
4 files changed, 144 insertions(+), 3 deletions(-)
diff --git a/src/protocol/pool_process_query.c b/src/protocol/pool_process_query.c
index 983a9c2a6..4e421e668 100644
--- a/src/protocol/pool_process_query.c
+++ b/src/protocol/pool_process_query.c
@@ -4203,6 +4203,7 @@ end_internal_transaction(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * back
PG_END_TRY();
POOL_SETMASK(&oldmask);
+ pool_unset_failed_transaction();
return POOL_CONTINUE;
}
diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c
index 01a4b13f4..fba1c542f 100644
--- a/src/protocol/pool_proto_modules.c
+++ b/src/protocol/pool_proto_modules.c
@@ -103,6 +103,8 @@ static void pool_discard_except_sync_and_ready_for_query(POOL_CONNECTION * front
POOL_CONNECTION_POOL * backend);
static void si_get_snapshot(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend, Node *node, bool tstate_check);
+static bool check_transaction_state_and_abort(char *query, Node *node, POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend);
+
/*
* This is the workhorse of processing the pg_terminate_backend function to
* make sure that the use of function should not trigger the backend node failover.
@@ -306,6 +308,19 @@ SimpleQuery(POOL_CONNECTION * frontend,
*/
pool_start_query(query_context, contents, len, node);
+ /*
+ * Check if the transaction is in abort status. If so, we do nothing
+ * and just return an error message to frontend, execpt for
+ * transaction commit or abort command.
+ */
+ if (check_transaction_state_and_abort(contents, node, frontend, backend) == false)
+ {
+ pool_ps_idle_display(backend);
+ pool_query_context_destroy(query_context);
+ pool_set_skip_reading_from_backends();
+ return POOL_CONTINUE;
+ }
+
/*
* Create PostgreSQL version cache. Since the provided query might
* cause a syntax error, we want to issue "SELECT version()" which is
@@ -2050,7 +2065,7 @@ ReadyForQuery(POOL_CONNECTION * frontend,
* transaction.
*/
/* if (pool_is_query_in_progress() && allow_close_transaction) */
- if (allow_close_transaction)
+ if (REPLICATION && allow_close_transaction)
{
bool internal_transaction_started = INTERNAL_TRANSACTION_STARTED(backend, MAIN_NODE_ID);
@@ -2065,8 +2080,9 @@ ReadyForQuery(POOL_CONNECTION * frontend,
}
/* close an internal transaction */
- if (end_internal_transaction(frontend, backend) != POOL_CONTINUE)
- return POOL_END;
+ if (internal_transaction_started)
+ if (end_internal_transaction(frontend, backend) != POOL_CONTINUE)
+ return POOL_END;
/*
* If we are running in snapshot isolation mode and started an
@@ -4516,3 +4532,51 @@ si_get_snapshot(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend, Node
si_snapshot_acquired();
}
}
+
+/*
+ * Check if the transaction is in abort status. If so, we do nothing and just
+ * return error message and ready for query message to frontend, then return
+ * false to caller.
+ */
+static bool
+check_transaction_state_and_abort(char *query, Node *node, POOL_CONNECTION * frontend,
+ POOL_CONNECTION_POOL * backend)
+{
+ int len;
+
+ /*
+ * Are we in failed transaction and the command is not a transaction close
+ * command?
+ */
+ if (pool_is_failed_transaction() && !is_commit_or_rollback_query(node))
+ {
+ StringInfoData buf;
+
+ initStringInfo(&buf);
+ appendStringInfo(&buf, "statement: %s", query);
+
+ /* send an error message to frontend */
+ pool_send_error_message(
+ frontend,
+ MAJOR(backend),
+ "25P02",
+ "current transaction is aborted, commands ignored until end of transaction block",
+ buf.data,
+ "",
+ __FILE__,
+ __LINE__);
+
+ pfree(buf.data);
+
+ /* send ready for query to frontend */
+ pool_write(frontend, "Z", 1);
+ len = 5;
+ len = htonl(len);
+ pool_write(frontend, &len, sizeof(len));
+ pool_write(frontend, "E", 1);
+ pool_flush(frontend);
+
+ return false;
+ }
+ return true;
+}
diff --git a/src/test/regression/tests/078.aborted_transaction/expected.txt b/src/test/regression/tests/078.aborted_transaction/expected.txt
new file mode 100644
index 000000000..be37381b4
--- /dev/null
+++ b/src/test/regression/tests/078.aborted_transaction/expected.txt
@@ -0,0 +1,28 @@
+BEGIN;
+BEGIN
+SELECT * FROM non_existing_table;
+ERROR: relation "non_existing_table" does not exist
+LINE 1: SELECT * FROM non_existing_table;
+ ^
+SELECT 1;
+ERROR: current transaction is aborted, commands ignored until end of transaction block
+DETAIL: statement: SELECT 1;
+END;
+ROLLBACK
+BEGIN;
+BEGIN
+aaa;
+ERROR: syntax error at or near "aaa"
+LINE 1: aaa;
+ ^
+SELECT 1;
+ERROR: current transaction is aborted, commands ignored until end of transaction block
+DETAIL: statement: SELECT 1;
+END;
+ROLLBACK
+SELECT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
diff --git a/src/test/regression/tests/078.aborted_transaction/test.sh b/src/test/regression/tests/078.aborted_transaction/test.sh
new file mode 100755
index 000000000..47bd280c1
--- /dev/null
+++ b/src/test/regression/tests/078.aborted_transaction/test.sh
@@ -0,0 +1,48 @@
+#!/usr/bin/env bash
+#-------------------------------------------------------------------
+# test script for aborted transaction.
+
+source $TESTLIBS
+TESTDIR=testdir
+PSQL=$PGBIN/psql
+PG_CTL=$PGBIN/pg_ctl
+
+rm -fr $TESTDIR
+mkdir $TESTDIR
+cd $TESTDIR
+
+# create test environment
+echo -n "creating test environment..."
+$PGPOOL_SETUP || exit 1
+echo "done."
+echo "backend_weight1 = 0" >> etc/pgpool.conf
+
+source ./bashrc.ports
+export PGPORT=$PGPOOL_PORT
+
+./startall
+wait_for_pgpool_startup
+
+$PSQL -e test > results.txt 2>&1 <<EOF
+BEGIN;
+SELECT * FROM non_existing_table;
+SELECT 1;
+END;
+BEGIN;
+aaa;
+SELECT 1;
+END;
+SELECT 1;
+EOF
+
+if cmp ../expected.txt results.txt >/dev/null
+then
+ echo "test succeeded"
+else
+ echo "test failed"
+ diff -c ../expected.txt results.txt
+ ./shutdownall
+ exit 1
+fi
+./shutdownall
+exit 0
[parent: 37d2c7ba0c00]