From d049ebbeeaf0fc27f84ab615c4c3bb9cb8462ab8 Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii Date: Thu, 30 Apr 2026 19:13:10 +0900 Subject: [PATCH v1] Fix to send sync message to necessary backend node. This is a follow-up commit for a350afd70. The commit fails to send sync messages to necessary backend node if previous query was generated by do_query. When do_query generates extended queries outsiede an explicit transaction, it does not send sync message because if it sends, unnamed portal may be closed. If nothing happen until session ends, when queries in reset_query_list are executed, typically including DISCARD ALL, FATAL error occurs (remeber that PostgreSQL opens a transaction while executing extended queries until sync message received). backend pid: 62624 statement: "DISCARD ALL" message: "DISCARD ALL cannot run inside a transaction block" 2026-04-29 01:00:58.288: pgbench pid 62605: WARNING: packet kind of backend 1 ['C'] does not match with main/majority nodes packet kind ['E'] 2026-04-29 01:00:58.288: pgbench pid 62605: FATAL: failed to read kind from backend 2026-04-29 01:00:58.288: pgbench pid 62605: DETAIL: kind mismatch among backends. Possible last query was: "DISCARD ALL" kind details are:0[E: DISCARD ALL cannot run inside a transaction block] 1[C] 2026-04-29 01:00:58.288: pgbench pid 62605: HINT: check data consistency among db nodes This was observed in 120.memory_leak_extended_memqcache test. Commit a350afd70 allows to send sync messages only to necessary backend nodes. Any queries comes from clients are tracked for this purpose. However it forgot to the case when do_query generates internal queries. We should have tracked such that queries, so that a sync message is issued to the server which do_query sent to, when the client sends a sync message. For this purpose I add a new member "bool pending_sync_map[MAX_NUM_BACKENDS]" to session_context, which a node id is set to when do_query sends a extended query to backend. Note that it is set only when do_query is called outside an explicit transaction. In add_sync_pending_message, which is called when client sends a sync message, we check the map and set sync_map accordingly so that subsequent call to SimpleForwardToBackend will send sync message to the necessary backend node. The commit also includes some tweaks to regression tests. 120.memory_leak_extended_memqcache: explicitely set backend_weight0 = 0 so that the load balance node is always 1. If load balance node is 0, the error above will not happen because when clients sends sync message, it is surely forwarded to 0, which do_query previously sent queries. 039.log_backend_messages: by the effect of the patch, trace of sending to node 0 is now included in the expected.s. This is normal. Author: Tatsuo Ishii Discussion: Backpatch-through: master --- src/include/context/pool_session_context.h | 6 ++++++ src/protocol/pool_process_query.c | 4 ++++ src/protocol/pool_proto_modules.c | 15 +++++++++++++++ .../tests/039.log_backend_messages/expected.s | 1 + .../120.memory_leak_extended_memqcache/test.sh | 2 ++ 5 files changed, 28 insertions(+) diff --git a/src/include/context/pool_session_context.h b/src/include/context/pool_session_context.h index 446357de3..eba56982b 100644 --- a/src/include/context/pool_session_context.h +++ b/src/include/context/pool_session_context.h @@ -347,6 +347,12 @@ typedef struct * A map to send sync message. Each entry represents backend node. */ bool sync_map[MAX_NUM_BACKENDS]; + + /* + * Backend node id map do_query sent to, without sending a sync + * message. + */ + bool pending_sync_map[MAX_NUM_BACKENDS]; } POOL_SESSION_CONTEXT; extern void pool_init_session_context(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend); diff --git a/src/protocol/pool_process_query.c b/src/protocol/pool_process_query.c index abf21fdce..dacaa9d5a 100644 --- a/src/protocol/pool_process_query.c +++ b/src/protocol/pool_process_query.c @@ -2117,7 +2117,11 @@ do_query(POOL_CONNECTION *backend, char *query, POOL_SELECT_RESULT **result, int if (backend->tstate == 'T') pool_write(backend, "S", 1); /* send "sync" message */ else + { pool_write(backend, "H", 1); /* send "flush" message */ + /* remember that we sent queries but did not send sync message */ + pool_get_session_context(true)->pending_sync_map[backend->db_node_id] = true; + } len = htonl(sizeof(len)); pool_write_and_flush(backend, &len, sizeof(len)); } diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c index f9458bb55..5072dd3a9 100644 --- a/src/protocol/pool_proto_modules.c +++ b/src/protocol/pool_proto_modules.c @@ -5362,6 +5362,21 @@ add_sync_pending_message(void) if (i == NUM_BACKENDS) memset(session_context->sync_map, true, NUM_BACKENDS); + /* + * Check if do_query sent queries but have not send sync message yet. If + * so, set the node id to sync_map so that a sync message is sent to the + * node. Otherwise, the transaction keeps open, which cause subsequent + * queries might fail: i.e. DISCARD ALL in reset_query_list. + */ + for (i = 0; i < NUM_BACKENDS; i++) + { + if (session_context->pending_sync_map[i]) + { + session_context->sync_map[i] = true; + session_context->pending_sync_map[i] = false; + } + } + /* copy sync map to query context's where_to_send map */ memcpy(query_context->where_to_send, session_context->sync_map, sizeof(query_context->where_to_send)); diff --git a/src/test/regression/tests/039.log_backend_messages/expected.s b/src/test/regression/tests/039.log_backend_messages/expected.s index a5ffeebec..5877fe361 100644 --- a/src/test/regression/tests/039.log_backend_messages/expected.s +++ b/src/test/regression/tests/039.log_backend_messages/expected.s @@ -88,6 +88,7 @@ FE=> Sync <= BE DataRow <= BE NoticeResponse(S LOG C XX000 M CommandComplete message from backend 1 <= BE CommandComplete(SELECT 3) +<= BE NoticeResponse(S LOG C XX000 M ReadyForQuery message from backend 0 <= BE NoticeResponse(S LOG C XX000 M ReadyForQuery message from backend 1 <= BE ReadyForQuery(I) FE=> Terminate diff --git a/src/test/regression/tests/120.memory_leak_extended_memqcache/test.sh b/src/test/regression/tests/120.memory_leak_extended_memqcache/test.sh index 4a2badb51..cfa284467 100755 --- a/src/test/regression/tests/120.memory_leak_extended_memqcache/test.sh +++ b/src/test/regression/tests/120.memory_leak_extended_memqcache/test.sh @@ -32,6 +32,8 @@ do # enable query cache echo "memory_cache_enabled = on" >> etc/pgpool.conf + # load balance node is 1 + echo "backend_weight0 = 0" >> etc/pgpool.conf # start pgpool-II ./startall -- 2.43.0