From d18584d2211de493c535f70488d1c226cc7e7f32 Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii Date: Thu, 4 Jun 2026 16:02:11 +0900 Subject: [PATCH v2] Fix disable_load_balance_on_write to not break query cache. The disable_load_balance_on_write accepts four options: transaction (the default) trans_transaction dml_adaptive always It appeared that except "transaction", all other options break query cache feature. Sometimes a query result is cached even there's a write query in a transaction, sometimes query is not cached even when it should be. The query cache relies on is_writing_transaction of session context to judge whether cache can be safely used. However, disable_load_balance_on_write overrides it to true when it should not, and vice versa for its own purpose. To fix this new session context variable "really_writing_transaction" is introduced. It is almost same as existing writing_transaction, but it faithfully tracks whether a writing query appears in an explicit transaction. The query cache uses it instead of writing_transaction variable. Add test cases for disable_load_balance_on_write = "trans_transaction", "dml_adaptive" and "always" to 006.memqcahce regression test. Note that "transaction" is the default and already tested in "s" mode. So it's not necessary to add a test for the case. Author: Tatsuo Ishii Discussion: Backpatch-through: v4.3 --- src/context/pool_session_context.c | 22 +++++++++++++++++++ src/include/context/pool_session_context.h | 19 +++++++++++++++- src/protocol/CommandComplete.c | 1 + src/protocol/pool_process_query.c | 1 + src/protocol/pool_proto_modules.c | 17 ++++++++++---- .../regression/tests/006.memqcache/test.sh | 19 +++++++++++++++- 6 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/context/pool_session_context.c b/src/context/pool_session_context.c index ded41c7fc..a87cce164 100644 --- a/src/context/pool_session_context.c +++ b/src/context/pool_session_context.c @@ -125,6 +125,7 @@ pool_init_session_context(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backe /* We don't have a write query in this transaction yet */ pool_unset_writing_transaction(); + pool_unset_really_writing_transaction(); /* Error doesn't occur in this transaction yet */ pool_unset_failed_transaction(); @@ -731,6 +732,12 @@ pool_unset_writing_transaction(void) } } +void +pool_unset_really_writing_transaction(void) +{ + pool_get_session_context(false)->really_writing_transaction = false; +} + /* * We have a write query in this transaction. */ @@ -749,6 +756,12 @@ pool_set_writing_transaction(void) } } +void +pool_set_really_writing_transaction(void) +{ + pool_get_session_context(false)->really_writing_transaction = true; +} + /* * Do we have a write query in this transaction? */ @@ -758,6 +771,15 @@ pool_is_writing_transaction(void) return pool_get_session_context(false)->writing_transaction; } +/* + * Do we really have a write query in this transaction? + */ +bool +pool_is_really_writing_transaction(void) +{ + return pool_get_session_context(false)->really_writing_transaction; +} + /* * Error doesn't occur in this transaction yet. */ diff --git a/src/include/context/pool_session_context.h b/src/include/context/pool_session_context.h index eba56982b..16e24613d 100644 --- a/src/include/context/pool_session_context.h +++ b/src/include/context/pool_session_context.h @@ -209,9 +209,23 @@ typedef struct /* If true, the command in progress has finished successfully. */ bool command_success; - /* If true, write query has been appeared in this transaction */ + /* + * If true, write query has been appeared in this transaction. Note that + * the flag may not be turned off even if a transaction is started or + * committed if disable_load_balance_on_write is other than "transaction". + * Also if disable_load_balance_on_write is "dml_adaptive", the flag is + * never be turned on. + */ bool writing_transaction; + /* + * Unlike "writing_transaction", this flag is turned on whenever writing + * query is issued in an explicit transaction, and is turned off when the + * transaction is closed. Of course turned off when new transaction + * starts. This flag is referenced by query cache. + */ + bool really_writing_transaction; + /* If true, error occurred in this transaction */ bool failed_transaction; @@ -384,8 +398,11 @@ extern void pool_set_sent_message_state(POOL_SENT_MESSAGE *message); extern void pool_zap_query_context_in_sent_messages(POOL_QUERY_CONTEXT *query_context); extern POOL_SENT_MESSAGE *pool_get_sent_message_by_query_context(POOL_QUERY_CONTEXT *query_context); extern void pool_unset_writing_transaction(void); +extern void pool_unset_really_writing_transaction(void); extern void pool_set_writing_transaction(void); +extern void pool_set_really_writing_transaction(void); extern bool pool_is_writing_transaction(void); +extern bool pool_is_really_writing_transaction(void); extern void pool_unset_failed_transaction(void); extern void pool_set_failed_transaction(void); extern bool pool_is_failed_transaction(void); diff --git a/src/protocol/CommandComplete.c b/src/protocol/CommandComplete.c index a3b8f0ea1..1f63a0e8d 100644 --- a/src/protocol/CommandComplete.c +++ b/src/protocol/CommandComplete.c @@ -370,6 +370,7 @@ handle_query_context(POOL_CONNECTION_POOL *backend) if (pool_config->disable_load_balance_on_write != DLBOW_TRANS_TRANSACTION) pool_unset_writing_transaction(); + pool_unset_really_writing_transaction(); pool_unset_failed_transaction(); pool_unset_transaction_isolation(); diff --git a/src/protocol/pool_process_query.c b/src/protocol/pool_process_query.c index dacaa9d5a..fdc8d97e0 100644 --- a/src/protocol/pool_process_query.c +++ b/src/protocol/pool_process_query.c @@ -4187,6 +4187,7 @@ start_internal_transaction(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *back /* Mark that we started new transaction */ INTERNAL_TRANSACTION_STARTED(backend, i) = true; pool_unset_writing_transaction(); + pool_unset_really_writing_transaction(); } } } diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c index 65ed190ef..86fb5f8a8 100644 --- a/src/protocol/pool_proto_modules.c +++ b/src/protocol/pool_proto_modules.c @@ -270,7 +270,7 @@ SimpleQuery(POOL_CONNECTION *frontend, * query cache. */ if (pool_config->memory_cache_enabled && is_likely_select && - !pool_is_writing_transaction() && + !pool_is_really_writing_transaction() && TSTATE(backend, MAIN_REPLICA ? PRIMARY_NODE_ID : REAL_MAIN_NODE_ID) != 'E' && !query_cache_disabled()) { @@ -1029,7 +1029,7 @@ Execute(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend, * message has 0 row argument, we maybe able to use cache. If * partial_fetch is true, cannot use cache. */ - if (pool_config->memory_cache_enabled && !pool_is_writing_transaction() && + if (pool_config->memory_cache_enabled && !pool_is_really_writing_transaction() && (TSTATE(backend, MAIN_REPLICA ? PRIMARY_NODE_ID : REAL_MAIN_NODE_ID) != 'E') && pool_is_likely_select(query) && !query_cache_disabled() && (query_context->atEnd || num_rows == 0) && @@ -1276,6 +1276,8 @@ Execute(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend, if (!pool_is_transaction_read_only(node)) { pool_set_writing_transaction(); + if (TSTATE(backend, MAIN_REPLICA ? PRIMARY_NODE_ID : REAL_MAIN_NODE_ID) == 'T') + pool_set_really_writing_transaction(); } } } @@ -4745,7 +4747,7 @@ pool_at_command_success(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend { if (pool_config->disable_load_balance_on_write != DLBOW_TRANS_TRANSACTION) pool_unset_writing_transaction(); - + pool_unset_really_writing_transaction(); pool_unset_failed_transaction(); pool_unset_transaction_isolation(); } @@ -4759,7 +4761,7 @@ pool_at_command_success(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend { if (pool_config->disable_load_balance_on_write != DLBOW_TRANS_TRANSACTION) pool_unset_writing_transaction(); - + pool_unset_really_writing_transaction(); pool_unset_failed_transaction(); pool_unset_transaction_isolation(); } @@ -4804,6 +4806,13 @@ pool_at_command_success(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend (errmsg("not SET TRANSACTION READ ONLY"))); pool_set_writing_transaction(); + + /* + * In case in transaction, we need to + * really_writing_transaction so that query cache is disabled. + */ + if (TSTATE(backend, MAIN_REPLICA ? PRIMARY_NODE_ID : REAL_MAIN_NODE_ID) == 'T') + pool_set_really_writing_transaction(); } } diff --git a/src/test/regression/tests/006.memqcache/test.sh b/src/test/regression/tests/006.memqcache/test.sh index f2371744d..6de458a4a 100755 --- a/src/test/regression/tests/006.memqcache/test.sh +++ b/src/test/regression/tests/006.memqcache/test.sh @@ -20,12 +20,25 @@ function del_details_from_error cat|sed -e '/ErrorResponse/s/ F .*//' -e '/NoticeResponse/s/ F .*$//' } -for mode in s r n +# s_dlbw* are tests for streaming replication mode + disable_load_balance_on_write +for mode in s r n \ + s_dlbw_trans_transaction \ + s_dlbw_dml_adaptive \ + s_dlbw_always do + echo "======== testing mode: $mode =========" rm -fr $TESTDIR mkdir $TESTDIR cd $TESTDIR + if [[ $mode =~ (s_dlbw_*) ]];then + opt=`echo $mode|sed s"/s_dlbw_//"` + echo $opt + mode="s" + else + opt="" + fi + # create test environment echo -n "creating test environment..." $PGPOOL_SETUP -m $mode -n 2 || exit 1 @@ -37,6 +50,10 @@ do echo "log_per_node_statement = on" >> etc/pgpool.conf echo "log_client_messages = on" >> etc/pgpool.conf echo "log_min_messages = debug5" >> etc/pgpool.conf + if [ "$opt" != "" ];then + echo "disable_load_balance_on_write = $opt" >> etc/pgpool.conf + echo "set disable_load_balance_on_write = $opt" + fi source ./bashrc.ports -- 2.43.0