dml_adaptive: ROLLBACK TO SAVEPOINT is treated as transaction end — write list is cleared and SELECTs on written tables are load balanced to standbys

[zourenli]

With disable_load_balance_on_write = dml_adaptive, pgpool-II tracks the tables written in an explicit transaction in POOL_SESSION_CONTEXT.transaction_temp_write_list, so that SELECTs touching those tables are routed to the primary instead of being load balanced to a standby (which cannot see the transaction's uncommitted changes).

ROLLBACK TO SAVEPOINT however is processed by dml_adaptive() as if the transaction had ended:

the accumulated transaction_temp_write_list is discarded,
session_context->is_in_transaction is reset to false,
(on master, in dml_adaptive_global mode, transaction_temp_write_oid_list is discarded as well).
The backend transaction of course remains open. From the ROLLBACK TO SAVEPOINT until the real COMMIT/ROLLBACK, read-your-own-writes inside the transaction is broken:

SELECTs on tables written before the savepoint are no longer pinned to the primary and can be load balanced to a standby.
Because is_in_transaction is false, the routing-side write list check is_select_object_in_temp_write_list() is disabled entirely, so the pin is lost for everything written after the savepoint rollback too.
This is a regression introduced by commit b86b776 ("Fix ROLLBACK TO command to work in aborted transaction.", 2022-11-25, back patched through 4.3). That commit — correctly — added is_rollback_to_query() to is_commit_or_rollback_query() so that ROLLBACK TO SAVEPOINT is allowed through the aborted-transaction gate in check_transaction_state_and_abort(). But dml_adaptive() consumes the same predicate in its TransactionStmt handling, and now mistakes ROLLBACK TO SAVEPOINT for COMMIT/ROLLBACK. Before that commit, ROLLBACK TO matched neither the start-transaction nor the commit/rollback branch in dml_adaptive() and the write list survived.

Version affected
Verified on current master (src/protocol/pool_process_query.c:1283, src/context/pool_query_context.c:2006).

Introduced by b86b776, which was back patched through 4.3 — so 4.3.x, 4.4.x, 4.5 and later are affected when running with disable_load_balance_on_write = dml_adaptive (and dml_adaptive_global on master). Not mode-specific otherwise: standard streaming replication setup with at least one standby.

Steps to reproduce
Setup: one primary + one standby, pgpool-II in streaming replication mode.

ini

pgpool.conf (relevant settings)

backend_hostname0 = 'primary-host'
backend_port0 = 5432
backend_weight0 = 0 # weight 0 on primary: every load-balanced
# statement deterministically goes to the standby
backend_hostname1 = 'standby-host'
backend_port1 = 5432
backend_weight1 = 1
disable_load_balance_on_write = dml_adaptive
log_per_node_statement = on
Prepare data (autocommit, replicates to the standby; wait until the standby has replayed it):

sql
CREATE TABLE t1 AS SELECT 1::int AS v;
Then in one psql session connected through pgpool-II:

sql
BEGIN;
UPDATE t1 SET v = 100;
SELECT v FROM t1; -- OK: returns 100 (routed to primary,
-- t1 is in the write list)

SAVEPOINT sp1;
UPDATE t1 SET v = 200;
ROLLBACK TO SAVEPOINT sp1; -- transaction is still open

SELECT v FROM t1; -- BUG: routed to the standby, returns 1
-- (stale: cannot see v=100)

UPDATE t1 SET v = 300; -- writes after the savepoint rollback are
SELECT v FROM t1; -- no longer pinned either: routed to the
-- standby, still returns 1

COMMIT;
With log_per_node_statement = on, the log shows the last two SELECTs executed on DB node 1 (standby) instead of DB node 0 (primary).

Expected behavior
ROLLBACK TO SAVEPOINT does not end the transaction. The write list and is_in_transaction must survive it, so that SELECTs on tables written in the (still open) transaction keep being routed to the primary. In the example above, both SELECTs after the ROLLBACK TO SAVEPOINT should return 100/300 from the primary.

Actual behavior
The ROLLBACK TO SAVEPOINT statement itself takes the transaction-end branch in dml_adaptive() (src/context/pool_query_context.c:2006), clearing transaction_temp_write_list and resetting is_in_transaction to false. Both SELECTs after it are load balanced to the standby and return stale data (1), even though the backend transaction on the primary is open and holds the updates.

Proposed fix (minimal)
Exclude ROLLBACK TO SAVEPOINT from the transaction-end branch of dml_adaptive(), restoring the pre-regression behavior (no-op in dml_adaptive(), write list untouched):

diff

--- a/src/context/pool_query_context.c
+++ b/src/context/pool_query_context.c
@@ -2006,7 +2006,16 @@ dml_adaptive(Node *node, char *query)

-			else if (is_commit_or_rollback_query(node))
+			/*
+			 * ROLLBACK TO SAVEPOINT does not end the transaction.  The
+			 * write list must survive it, otherwise SELECTs on tables
+			 * written in this transaction are load balanced to standbys,
+			 * which cannot see the uncommitted changes.  Keeping the full
+			 * accumulated list is conservative (tables written after the
+			 * savepoint stay pinned to the primary until COMMIT/ROLLBACK)
+			 * but never produces stale reads.
+			 */
+			else if (is_commit_or_rollback_query(node) && !is_rollback_to_query(node))
 			{
 				session_context->is_in_transaction = false;

Notes:

This deliberately keeps the entire accumulated write list rather than restoring the list snapshot taken at SAVEPOINT time. Tables written after the savepoint and then rolled back therefore remain pinned to the primary until the transaction ends — an over-approximation that only costs some load balancing opportunities, never correctness. (A possible future refinement would be to snapshot the write list on SAVEPOINT and restore it on ROLLBACK TO SAVEPOINT.)
For dml_adaptive_global on master, the same exclusion also keeps transaction_temp_write_oid_list across ROLLBACK TO SAVEPOINT; OIDs of tables whose post-savepoint writes were rolled back may then still be marked in shared memory at COMMIT — again the safe (over-invalidation) direction.
The aborted-transaction behavior fixed by b86b776 is not affected: check_transaction_state_and_abort() keeps seeing ROLLBACK TO SAVEPOINT through the unchanged is_commit_or_rollback_query().
Additional context
Root-cause predicate: is_commit_or_rollback_query() at src/protocol/pool_process_query.c:1283-1286 (is_commit_query(node) || is_rollback_query(node) || is_rollback_to_query(node)).
Routing-side consumer that loses the pin: is_select_object_in_temp_write_list() at src/context/pool_query_context.c:1852, called from the SELECT routing path at src/context/pool_query_context.c:2377.
Related but distinct open issues checked (not duplicates): #157 (dml_adaptive CREATE TABLE routing in transaction) and the 25P02 synthesized-error report on ROLLBACK TO SAVEPOINT in aborted transactions.

[koshino-235]

Hi,

Thank you for the report and proposed fix.

I was able to reproduce the issue. After ROLLBACK TO SAVEPOINT, SELECTs
on the previously modified table were sent to the standby and returned
stale values.

I made a minor change to the proposed patch for readability. Instead of
excluding ROLLBACK TO SAVEPOINT from is_commit_or_rollback_query(), the
transaction-end branch now explicitly checks for COMMIT or full
ROLLBACK:

is_commit_query(node) || is_rollback_query(node)

This makes it clear which queries clear the transaction state and write
list, while ROLLBACK TO SAVEPOINT preserves them.

After applying the change, the SELECTs after ROLLBACK TO SAVEPOINT were
sent to the primary and returned the expected values.

I have attached the detailed reproduction and verification results in issue_173.txt.

v2-0001-Preserve-dml_adaptive-state-on-ROLLBACK-TO-SAVEPO.patch

issue_173.txt

[zourenli]

Thanks for the quick follow‑up and for taking the time to reproduce this. I also appreciate the readability improvement – checking COMMIT / full ROLLBACK explicitly is cleaner and makes the intent obvious.

I've reviewed the v2 patch and the verification results in issue_173.txt. The updated behaviour correctly routes SELECT queries to the primary after ROLLBACK TO SAVEPOINT, and I can confirm that the stale‑data issue is fully resolved.

No further concerns from my side. Let me know if you'd like me to run any additional tests or provide more environment details. Otherwise, I'm happy to see this merged.

Thanks again!