Skip to content

Skip coordinated transactions (2PC) for single-statement, single-shard procedure calls#8524

Merged
colm-mchugh merged 8 commits into
citusdata:mainfrom
paragikjain:users/paragjain/sprocOptimizationForSingleShard
Mar 26, 2026
Merged

Skip coordinated transactions (2PC) for single-statement, single-shard procedure calls#8524
colm-mchugh merged 8 commits into
citusdata:mainfrom
paragikjain:users/paragjain/sprocOptimizationForSingleShard

Conversation

@paragikjain
Copy link
Copy Markdown
Contributor

citus.enable_single_shard_procedure_optimization (default off) that allows stored procedure calls containing a single distributed write targeting one shard with one placement to skip coordinated (2PC) transactions. This eliminates the overhead of BEGIN → PREPARE TRANSACTION → COMMIT PREPARED for the common case of single-statement, single-shard procedures.

Important partial-commit behavior: Because the first statement executes without 2PC, it auto-commits on the worker. If the procedure contains a second distributed statement, the ERROR prevents silent data inconsistency — but the first statement's effects are already committed and cannot be rolled back. This is explicitly documented in the GUC description, function comments, and the error message's DETAIL line

@paragikjain paragikjain marked this pull request as ready for review March 18, 2026 12:27
Comment thread src/test/regress/sql/single_shard_procedure_optimization.sql
Comment thread src/test/regress/sql/single_shard_procedure_optimization.sql Outdated
Comment thread src/test/regress/sql/single_shard_procedure_optimization.sql
Comment thread src/backend/distributed/shared_library_init.c Outdated
Comment thread src/backend/distributed/shared_library_init.c Outdated
Comment thread src/test/regress/sql/single_shard_procedure_optimization.sql Outdated
Comment thread src/backend/distributed/executor/executor_util_tasks.c Outdated
Comment thread src/backend/distributed/executor/executor_util_tasks.c
Comment thread src/test/regress/sql/single_shard_procedure_optimization.sql
Copy link
Copy Markdown
Contributor

@colm-mchugh colm-mchugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm @paragikjain , thanks for addressing review comments. Approving to enable merge, but there is one necessary step to enable check-style to pass; run citus_indent --check in root dir of your citus repo and commit whatever files are flagged.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 19, 2026

Codecov Report

❌ Patch coverage is 22.22222% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.80%. Comparing base (347d723) to head (39d24a6).
⚠️ Report is 1 commits behind head on main.

❌ Your patch status has failed because the patch coverage (22.22%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (44.80%) is below the target coverage (87.50%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (347d723) and HEAD (39d24a6). Click for more details.

HEAD has 91 uploads less than BASE
Flag BASE (347d723) HEAD (39d24a6)
17_citus_upgrade 1 0
16_citus_upgrade 1 0
18_regress_check-query-generator 1 0
18_regress_check-enterprise-failure 1 0
17_regress_check-pytest 1 0
16_regress_check-pytest 1 0
18_regress_check-pytest 1 0
17_regress_check-query-generator 1 0
17_regress_check-enterprise-failure 1 0
16_18_upgrade 1 0
17_18_upgrade 1 0
16_17_upgrade 1 0
16_regress_check-follower-cluster 1 0
17_regress_check-follower-cluster 1 0
16_regress_check-tap 1 0
18_regress_check-follower-cluster 1 0
18_regress_check-add-backup-node 1 0
18_regress_check-failure 1 0
17_regress_check-columnar-isolation 1 0
16_regress_check-enterprise-isolation-logicalrep-2 1 0
17_regress_check-columnar 1 0
18_regress_check-enterprise-isolation-logicalrep-2 1 0
17_regress_check-split 1 0
18_regress_check-columnar 1 0
16_regress_check-columnar-isolation 1 0
17_regress_check-enterprise-isolation-logicalrep-2 1 0
18_regress_check-enterprise-isolation-logicalrep-3 1 0
17_regress_check-enterprise-isolation-logicalrep-3 1 0
16_regress_check-enterprise-failure 1 0
17_regress_check-tap 1 0
17_regress_check-add-backup-node 1 0
16_regress_check-add-backup-node 1 0
16_regress_check-vanilla 1 0
17_regress_check-failure 1 0
18_regress_check-columnar-isolation 1 0
18_regress_check-tap 1 0
17_regress_check-vanilla 1 0
16_cdc_installcheck 1 0
16_regress_check-multi-mx 1 0
18_regress_check-split 1 0
16_regress_check-enterprise-isolation 1 0
18_regress_check-enterprise-isolation 1 0
18_regress_check-enterprise-isolation-logicalrep-1 1 0
16_regress_check-split 1 0
16_regress_check-multi-1-create-citus 1 0
16_regress_check-enterprise-isolation-logicalrep-3 1 0
16_regress_check-query-generator 1 0
16_regress_check-enterprise-isolation-logicalrep-1 1 0
17_regress_check-multi-1-create-citus 1 0
18_regress_check-vanilla 1 0
17_regress_check-enterprise-isolation-logicalrep-1 1 0
18_regress_check-multi-mx 1 0
18_regress_check-multi-1-create-citus 1 0
17_regress_check-multi-mx 1 0
16_regress_check-columnar 1 0
16_regress_check-operations 1 0
17_regress_check-operations 1 0
18_regress_check-isolation 1 0
17_regress_check-isolation 1 0
16_regress_check-isolation 1 0
16_arbitrary_configs_5 1 0
16_regress_check-enterprise 1 0
18_regress_check-multi 1 0
16_arbitrary_configs_1 1 0
18_arbitrary_configs_3 1 0
16_arbitrary_configs_3 1 0
17_arbitrary_configs_3 1 0
18_arbitrary_configs_1 1 0
16_regress_check-failure 1 0
18_regress_check-operations 1 0
18_cdc_installcheck 1 0
18_arbitrary_configs_5 1 0
17_arbitrary_configs_5 1 0
18_arbitrary_configs_2 1 0
17_arbitrary_configs_2 1 0
16_regress_check-multi 1 0
17_regress_check-multi 1 0
16_arbitrary_configs_2 1 0
18_arbitrary_configs_4 1 0
16_arbitrary_configs_4 1 0
18_regress_check-multi-1 1 0
16_regress_check-multi-1 1 0
17_arbitrary_configs_4 1 0
18_arbitrary_configs_0 1 0
17_arbitrary_configs_0 1 0
17_regress_check-multi-1 1 0
16_arbitrary_configs_0 1 0
18_regress_check-enterprise 1 0
17_regress_check-enterprise 1 0
17_regress_check-enterprise-isolation 1 0
17_cdc_installcheck 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #8524       +/-   ##
===========================================
- Coverage   88.92%   44.80%   -44.12%     
===========================================
  Files         286      286               
  Lines       63179    62200      -979     
  Branches     7922     7644      -278     
===========================================
- Hits        56179    27866    -28313     
- Misses       4731    31893    +27162     
- Partials     2269     2441      +172     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/backend/distributed/executor/executor_util_tasks.c
@colm-mchugh colm-mchugh force-pushed the users/paragjain/sprocOptimizationForSingleShard branch from 1f50422 to 39d24a6 Compare March 25, 2026 15:35
@colm-mchugh colm-mchugh merged commit 029f381 into citusdata:main Mar 26, 2026
420 of 426 checks passed
codeforall added a commit that referenced this pull request Apr 12, 2026
Replace the runtime counter approach from PR #8524 with pre-execution
static analysis of procedure bodies using the PLpgSQL plugin func_beg
hook. This avoids partial commits followed by ERROR that the original
design could produce for multi-statement procedures.

Key changes:
- New procedure_body_analysis.c: PLpgSQL plugin that walks the statement
  tree at func_beg time, counting SQL-producing statements (EXECSQL,
  PERFORM, DYNEXECUTE, CALL) and detecting disqualifiers (COMMIT,
  ROLLBACK, and all loop types).
- Loops (FOR, WHILE, LOOP, FOREACH) are treated as disqualifiers since
  they can execute SQL multiple times, risking partial commits.
- Multi-statement procedures fall back gracefully to normal coordinated
  transactions (2PC) instead of raising an ERROR.
- Remove ProcedureNonCoordinatedExecutionCount global counter and its
  three reset sites in utility_hook.c.
- Update tests
Replace the runtime counter approach from PR #8524 with pre-execution
static analysis of procedure bodies using the PLpgSQ.
colm-mchugh pushed a commit that referenced this pull request Apr 29, 2026
Replace the runtime counter approach from PR #8524 with pre-execution
static analysis of procedure bodies using the PLpgSQL plugin func_beg
hook. This avoids partial commits followed by ERROR that the original
design could produce for multi-statement procedures.

Key changes:
- New procedure_body_analysis.c: PLpgSQL plugin that walks the statement
  tree at func_beg time, counting SQL-producing statements (EXECSQL,
  PERFORM, DYNEXECUTE, CALL) and detecting disqualifiers (COMMIT,
  ROLLBACK, and all loop types).
- Loops (FOR, WHILE, LOOP, FOREACH) are treated as disqualifiers since
  they can execute SQL multiple times, risking partial commits.
- Multi-statement procedures fall back gracefully to normal coordinated
  transactions (2PC) instead of raising an ERROR.
- Remove ProcedureNonCoordinatedExecutionCount global counter and its
  three reset sites in utility_hook.c.
- Update tests
Replace the runtime counter approach from PR #8524 with pre-execution
static analysis of procedure bodies using the PLpgSQ.

Adds a temporary benchmarking script, for testing/dev purposes only
(will not be in final commit)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants