Track Variable State Across Transactions and Savepoints - v3.0 by rahim-kanji · Pull Request #4929 · sysown/proxysql

rahim-kanji · 2025-04-30T10:06:28Z

High-Level Workflow

The PgSQL_ExplicitTxnStateMgr manages explicit PostgreSQL transactions and savepoints by tracking the state of session variables. Here's how it operates logically:

1. Transaction Start (BEGIN)

Snapshot Capture:

When a transaction begins, the manager takes a snapshot of all tracked session variables from client connection.

This includes their current values and a hash for integrity checks.
The snapshot is stored as the initial state of the transaction.

2. Creating Savepoints (SAVEPOINT)

State Preservation:

Each SAVEPOINT command triggers a new snapshot of the current session variables.

The snapshot is linked to the savepoint name (e.g., SAVEPOINT A creates snapshot A).
These snapshots form a stack, allowing nested rollbacks.

3. Committing (COMMIT)

Finalization:

The manager discards all snapshots and savepoints associated with the transaction.
The database changes are made permanent, and the transaction ends.

4. Rolling Back (ROLLBACK)

Full Reset:###

The manager reverts all session variables to their initial snapshot (captured at BEGIN).
All intermediate snapshots and savepoints are discarded.

5. Rolling Back to a Savepoint (ROLLBACK TO SAVEPOINT)

Targeted Reset:

The manager locates the snapshot linked to the specified savepoint (e.g., SAVEPOINT A).
Session variables are reset to match this snapshot.
All snapshots and savepoints created after the target are discarded.

Example: After SAVEPOINT A → SAVEPOINT B, rolling back to A removes B and its snapshot.

6. Releasing Savepoints (RELEASE SAVEPOINT)

Cleanup Without Rollback:

The specified savepoint and its snapshot are removed from the stack.
Unlike ROLLBACK TO, this does not revert variables—it simply prunes the transaction history.

Edge Cases & Safety

No Active Transaction:

Commands like COMMIT or ROLLBACK with no active transaction log warnings but take no action.

Duplicate Savepoints:

Creating a savepoint with an existing name is blocked to avoid ambiguity.

Debug Checks:

In debug builds, the manager verifies that server-side variables match client expectations to ensure consistency.

Example Workflow

1. BEGIN → Initial snapshot taken.  
2. SET variable X = 100 → Modify variable.  
3. SAVEPOINT A → Snapshot A (X=100).  
4. SET variable X = 200 → Modify again.  
5. SAVEPOINT B → Snapshot B (X=200).  
6. ROLLBACK TO A → Revert to Snapshot A (X=100), discard Snapshot B.  
7. COMMIT → Clear all snapshots.

Note:
Currently, SET LOCAL is not supported and will lock session on host group if executed.

Closes #4907

…haracters at compile time

Added get_pg_parameter_status

Dump transaction state in proxysql internal session output

Copilot

Pull Request Overview

This PR introduces explicit tracking of variable states across transactions and savepoints through a new transaction state manager and related updates in variable handling and query processing.

Updated client/server reset methods to take a flag (reorder_dynamic_variables_idx) for dynamic variable reordering
Integrated a new PgSQL_ExplicitTxnStateMgr class for managing transaction state and savepoint operations
Modified query parsing and connection handling functions to account for transaction state changes

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
lib/PgSQL_Variables.cpp	Added new parameter to reset functions and conditionally reordering indices
lib/PgSQL_Session.cpp	Instantiated and used the new transaction state manager
lib/PgSQL_Query_Processor.cpp	Updated tokenization and command parsing (including ABORT command handling)
lib/PgSQL_Protocol.cpp	Forwarded transaction state to function calls
lib/PgSQL_ExplicitTxnStateMgr.cpp	Introduced explicit transaction state management and savepoint support
lib/PgSQL_Connection.cpp	Modified query processing logic to use savepoint count
lib/MySQL_Session.cpp	Minor cleanup in error handling
lib/Base_Session.cpp	Adjusted transaction state handling with conditional MySQL-specific checks
include/proxysql_utils.h	Updated set_thread_name to use a templated approach
include/proxysql_structs.h	Added new PGSQL_QUERY_ABORT command
include/PgSQL_Variables.h	Updated method signatures to reflect new parameter usage
include/PgSQL_Session.h	Added transaction_state_manager member
include/PgSQL_Protocol.h	Updated related friend function declarations
include/PgSQL_ExplicitTxnStateMgr.h	Added declarations for the explicit transaction state manager

Files not reviewed (1)

lib/Makefile: Language not supported

renecannao · 2025-05-05T15:10:10Z

/gemini review

gemini-code-assist

Code Review

The pull request introduces transaction state management and savepoint functionality for PostgreSQL connections in ProxySQL. This is a significant enhancement that could improve the handling of complex transactions. However, the code requires careful review to ensure correctness, security, and maintainability.

Summary of Findings

Missing Abort Handling: The addition of PGSQL_QUERY_ABORT in proxysql_structs.h and PgSQL_Query_Processor.cpp is good, but the code in PgSQL_ExplicitTxnStateMgr.cpp doesn't explicitly handle the ABORT command. This could lead to inconsistent transaction state management.
Savepoint Logic: The logic for handling savepoints in PgSQL_Connection.cpp seems complex and potentially error-prone. The interaction between savepoint_count and IsKnownActiveTransaction needs careful consideration.
Potential Memory Leaks: The strdup calls in PgSQL_ExplicitTxnStateMgr.cpp require corresponding free calls to prevent memory leaks, especially in error handling paths.
Missing Error Handling: The async_connect function in PgSQL_Connection.cpp has cases for ASYNC_CONNECT_FAILED and ASYNC_CONNECT_TIMEOUT that return error codes, but the default case does not handle errors. This could lead to unhandled connection errors.

Merge Readiness

The pull request is not yet ready for merging. The issues related to missing abort handling, savepoint logic, potential memory leaks, and missing error handling need to be addressed before merging. I am unable to directly approve this pull request, and strongly recommend that others review and approve this code before merging, especially after the identified issues are resolved.

lib/PgSQL_Query_Processor.cpp

lib/PgSQL_Connection.cpp

rahim-kanji added 10 commits April 30, 2025 13:26

Added explicit transaction state manager

ae1f312

Added Abort (alias of Rollback)

2f9bb83

Added PgSQL_ExplicitTxnStateMgr in MakeFile

d095227

Added a static_assert to enforce a maximum thread name length of 16 c…

33b8b2b

…haracters at compile time

Added reorder_dynamic_variables_idx flag for server connection variables

c6fd3ce

Optimized savepoint detection

d90bf4a

Added get_pg_parameter_status

Added Transaction State flag in SQLite3_to_Postgres

190e369

Added PgSQL_ExplicitTxnStateMgr in session

979b3a8

Dump transaction state in proxysql internal session output

Some optimisation

9910a18

Added newline

afbd127

rahim-kanji changed the title ~~rack Variable State Across Transactions and Savepoints [WIP] - v3.0~~ Track Variable State Across Transactions and Savepoints [WIP] - v3.0 Apr 30, 2025

renecannao requested a review from Copilot May 5, 2025 12:20

Copilot AI reviewed May 5, 2025

View reviewed changes

gemini-code-assist bot suggested changes May 5, 2025

View reviewed changes

lib/PgSQL_Query_Processor.cpp Show resolved Hide resolved

lib/PgSQL_Connection.cpp Show resolved Hide resolved

rahim-kanji added 6 commits May 6, 2025 11:00

Added names (alias of client_encoding)

c71aa55

Removed extra semicolon

9559703

Few improvements

7c7cbd0

Improved set parser. SET LOCAL will now lock on hostgroup.

0adf56e

Added TAP test

f87fc30

Improved TAP test logging

4d5ca5a

rahim-kanji changed the title ~~Track Variable State Across Transactions and Savepoints [WIP] - v3.0~~ Track Variable State Across Transactions and Savepoints - v3.0 May 6, 2025

renecannao merged commit 9f4b139 into v3.0 May 7, 2025
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track Variable State Across Transactions and Savepoints - v3.0#4929

Track Variable State Across Transactions and Savepoints - v3.0#4929
renecannao merged 16 commits intov3.0from
v3.0_track_transaction_param_state_4907

rahim-kanji commented Apr 30, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

renecannao commented May 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

rahim-kanji commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High-Level Workflow

1. Transaction Start (BEGIN)

Snapshot Capture:

2. Creating Savepoints (SAVEPOINT)

State Preservation:

3. Committing (COMMIT)

Finalization:

4. Rolling Back (ROLLBACK)

5. Rolling Back to a Savepoint (ROLLBACK TO SAVEPOINT)

Targeted Reset:

6. Releasing Savepoints (RELEASE SAVEPOINT)

Cleanup Without Rollback:

Edge Cases & Safety

No Active Transaction:

Duplicate Savepoints:

Debug Checks:

Example Workflow

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

renecannao commented May 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rahim-kanji commented Apr 30, 2025 •

edited

Loading