Skip to content

Redesign procedure 2PC skip: static analysis via PLpgSQL plugin (Backport PR#8566 to 14.2) - #8694

Merged
Muhammad Usama (codeforall) merged 2 commits into
citusdata:release-14.0from
codeforall:muusama/bp-PR-8566-14.2
Jul 24, 2026
Merged

Redesign procedure 2PC skip: static analysis via PLpgSQL plugin (Backport PR#8566 to 14.2)#8694
Muhammad Usama (codeforall) merged 2 commits into
citusdata:release-14.0from
codeforall:muusama/bp-PR-8566-14.2

Conversation

@codeforall

@codeforall Muhammad Usama (codeforall) commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

DESCRIPTION: Avoid coordinated transactions for single-statement single-shard procedure calls

Replace the runtime-counter approach from PR#8524 with pre-execution static analysis of procedure bodies using the PLpgSQL plugin func_beg hook. The prior design detected multi-statement procedures only after the first statement had run, which could produce a partial commit followed by an ERROR. Analyzing the body before any statement executes lets multi-statement procedures fall back cleanly to a coordinated (2PC) transaction with no error and no partial commit.

How it works
A new PLpgSQL plugin walks the statement tree at func_beg time, before any statement runs and sets the ProcedureBodyIsSingleStatement flag. The executor gate (CanSkipProcedureCoordination) then treats a single-statement body as necessary but not sufficient: the existing single-task / single-placement / coordinated-transaction checks still apply before a skip is allowed.

The walker:

Counts SQL-producing statements: EXECSQL, PERFORM, DYNEXECUTE, and CALL (the motivating use case).
Disqualifies on COMMIT, ROLLBACK, and every loop type (LOOP, WHILE, FOR/FORI/FORS/FORC, DYNFORS, FOREACH), a statement inside a loop can run many times, which is exactly the partial-commit risk we avoid.
Recurses into container statements: a BLOCK contributes the sum of its body and its exception-handler actions (both may run), while an IF or CASE contributes the maximum SQL count across its branches (then/ELSIF/else, or WHEN/else) because only one branch executes at runtime. A disqualifier in any branch still disqualifies the whole body.

Key changes

New [procedure_body_analysis.c] PLpgSQL plugin performing the pre-execution walk.
Max-based IF/CASE branch counting, so a procedure with a single SQL statement per branch stays eligible for the skip.
Multi-statement or disqualified procedures fall back to normal coordinated (2PC) transactions instead of raising an ERROR.
Removed the ProcedureNonCoordinatedExecutionCount global counter and its reset sites in utility_hook.c; the single-statement flag is now also defensively reset on the procedure error path so it can never survive a failed procedure.
The optimization stays behind the off-by-default citus.enable_procedure_transaction_skip GUC (kept off here to keep the change backport-friendly for 14.2 & 13.4; enabling-by-default / GUC removal is deferred to a follow-up on main). Note the analysis only applies to PLpgSQL procedures.
Regression coverage for the IF, CASE, nested-block, and exception walker paths (plus the single-CALL and loop cases), asserting the 2PC skip via citus.log_remote_commands.

Parag Jain (paragikjain) and others added 2 commits July 23, 2026 15:20
…d procedure calls (citusdata#8524)

DESCRIPTION: relax transaction requirement for procedure calls with single statement.

Opt-in via 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

---------

Co-authored-by: Parag Jain <paragjain@microsoft.com>
(cherry picked from commit 029f381)
…sdata#8566)

DESCRIPTION: Avoid coordinated transactions for single-statement
single-shard procedure calls

Replace the runtime-counter approach from PR#8524 with pre-execution
static analysis of procedure bodies using the PLpgSQL plugin func_beg
hook. The prior design detected multi-statement procedures only after
the first statement had run, which could produce a partial commit
followed by an ERROR. Analyzing the body before any statement executes
lets multi-statement procedures fall back cleanly to a coordinated (2PC)
transaction with no error and no partial commit.

**How it works**
A new PLpgSQL plugin walks the statement tree at func_beg time, before
any statement runs and sets the ProcedureBodyIsSingleStatement flag. The
executor gate (CanSkipProcedureCoordination) then treats a
single-statement body as necessary but not sufficient: the existing
single-task / single-placement / coordinated-transaction checks still
apply before a skip is allowed.

**The walker:**

Counts SQL-producing statements: EXECSQL, PERFORM, DYNEXECUTE, and CALL
(the motivating use case).
Disqualifies on COMMIT, ROLLBACK, and every loop type (LOOP, WHILE,
FOR/FORI/FORS/FORC, DYNFORS, FOREACH), a statement inside a loop can run
many times, which is exactly the partial-commit risk we avoid.
Recurses into container statements: a BLOCK contributes the sum of its
body and its exception-handler actions (both may run), while an IF or
CASE contributes the maximum SQL count across its branches
(then/ELSIF/else, or WHEN/else) because only one branch executes at
runtime. A disqualifier in any branch still disqualifies the whole body.

**Key changes**

- New [procedure_body_analysis.c] PLpgSQL plugin performing the
pre-execution walk.
- Max-based IF/CASE branch counting, so a procedure with a single SQL
statement per branch stays eligible for the skip.
- Multi-statement or disqualified procedures fall back to normal
coordinated (2PC) transactions instead of raising an ERROR.
- Removed the ProcedureNonCoordinatedExecutionCount global counter and
its reset sites in utility_hook.c; the single-statement flag is now also
defensively reset on the procedure error path so it can never survive a
failed procedure.
- Regression coverage

(cherry picked from commit 2d179e2)
@codeforall Muhammad Usama (codeforall) changed the title Redesign procedure 2PC skip: static analysis via PLpgSQL plugin Backport PR#8566 to 14.2) Redesign procedure 2PC skip: static analysis via PLpgSQL plugin (Backport PR#8566 to 14.2) Jul 23, 2026
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.21705% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.58%. Comparing base (f70aeb5) to head (73c74ff).

Additional details and impacted files
@@               Coverage Diff                @@
##           release-14.0    #8694      +/-   ##
================================================
- Coverage         88.62%   88.58%   -0.04%     
================================================
  Files               288      289       +1     
  Lines             64375    64503     +128     
  Branches           8087     8107      +20     
================================================
+ Hits              57050    57140      +90     
- Misses             4991     5024      +33     
- Partials           2334     2339       +5     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codeforall
Muhammad Usama (codeforall) merged commit 97f95f2 into citusdata:release-14.0 Jul 24, 2026
391 of 404 checks passed
ibrahim halatci (ihalatci) pushed a commit that referenced this pull request Aug 5, 2026
Release prep for **citus v14.2.0** from `release-14.0`.

DESCRIPTION: add changelog for 14.2.0

All version-bearing files (configure.ac, configure, citus.control,
citus_columnar.control, multi_extension.out and the 14.1-1 -> 14.2-1
migration scripts) were already bumped in 152fdf4 (#8635), so this is a
single-file CHANGELOG addition.

### Changelog entries (user-visible only)

* #8621 - citus_internal.distribute_object() repair UDF
* #8625 - citus.allow_unsafe_insert_select_pushdown GUC
* #8566 (via #8694) - skip 2PC for single-statement single-shard procedures
* #8587 - object ownership for more citus-internal UDFs
* #8638 (via #8653) - re-range sequences when promoting a clone node
* #8651 (via #8656) - drop orphaned Citus local table shard copies
* #8497 - wrong results when recursive planning projects columns as NULL
* #8692 (via #8700) - UPDATE/DELETE with shard key equality + false predicate
* #8594 (via #8714) - race condition in shard cleanup
* #8498 (via #8715) - type mismatch with COLLATE + type cast
* #8556 (via #8716) - segfault in EXPLAIN with LEFT JOIN + subqueries
* #8465 (via #8718) - CREATE EXTENSION IF NOT EXISTS ownership error
* #8561 (via #8717) - crash on writable standby coordinator

CI/test-infra, SQL-plumbing and version-bump commits intentionally excluded.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
ibrahim halatci (ihalatci) pushed a commit that referenced this pull request Aug 6, 2026
Release prep for **citus v14.2.0** from `release-14.0`.

DESCRIPTION: add changelog for 14.2.0

All version-bearing files (configure.ac, configure, citus.control,
citus_columnar.control, multi_extension.out and the 14.1-1 -> 14.2-1
migration scripts) were already bumped in 152fdf4 (#8635), so this is a
single-file CHANGELOG addition.

### Changelog entries (user-visible only)

* #8621 - citus_internal.distribute_object() repair UDF
* #8625 - citus.allow_unsafe_insert_select_pushdown GUC
* #8566 (via #8694) - skip 2PC for single-statement single-shard procedures
* #8587 - object ownership for more citus-internal UDFs
* #8638 (via #8653) - re-range sequences when promoting a clone node
* #8651 (via #8656) - drop orphaned Citus local table shard copies
* #8497 - wrong results when recursive planning projects columns as NULL
* #8692 (via #8700) - UPDATE/DELETE with shard key equality + false predicate
* #8594 (via #8714) - race condition in shard cleanup
* #8498 (via #8715) - type mismatch with COLLATE + type cast
* #8556 (via #8716) - segfault in EXPLAIN with LEFT JOIN + subqueries
* #8465 (via #8718) - CREATE EXTENSION IF NOT EXISTS ownership error
* #8561 (via #8717) - crash on writable standby coordinator

CI/test-infra, SQL-plumbing and version-bump commits intentionally excluded.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
ibrahim halatci (ihalatci) added a commit that referenced this pull request Aug 6, 2026
Release prep for **citus v14.2.0** from `release-14.0`.

This is a **CHANGELOG-only** change. All version-bearing artifacts were
already
bumped in 152fdf4 (#8635):

| artifact | state on `release-14.0` |
| --- | --- |
| `configure.ac` / `configure` | `14.2.0` |
| `citus.control` / `citus_columnar.control` | `14.2-1` |
| `expected/multi_extension.out` | references `14.2` |
| `14.1-1` -> `14.2-1` migration + downgrade scripts | present |

So the diff here is a single file: `CHANGELOG.md | 48 +`.

### Changelog entries (user-visible only)

| origin PR | backport PR | entry |
| --- | --- | --- |
| #8621 | - | `citus_internal.distribute_object()` repair UDF |
| #8625 | - | `citus.allow_unsafe_insert_select_pushdown` GUC |
| #8566 | #8694 | skip 2PC for single-statement single-shard procedures
|
| #8587 | - | object ownership for more citus-internal UDFs |
| #8638 | #8653 | re-range sequences when promoting a clone node |
| #8651 | #8656 | drop orphaned Citus local table shard copies |
| #8497 | - | wrong results when recursive planning projects columns as
NULL |
| #8692 | #8700 | `UPDATE`/`DELETE` with dist-key equality +
always-false predicate |
| #8594 | #8714 | race condition in shard cleanup (stale catalog
snapshot) |
| #8498 | #8715 | type mismatch with `COLLATE` + type cast |
| #8556 | #8716 | segfault in `EXPLAIN` with `LEFT JOIN` + correlated
subqueries |
| #8465 | #8718 | `CREATE EXTENSION IF NOT EXISTS` ownership error |
| #8561 | #8717 | crash on a writable standby coordinator |

Bullets cite **origin** PR numbers only, per the existing convention in
the file.

### Commits intentionally excluded

Four of the 17 commits in `v14.1.0..release-14.0` carry no
`DESCRIPTION:` line
and are not user-visible:

- `f70aeb54f` - move a GUC test into the N-1-excluded schedule (folds
into #8625)
- `3383b2463` - `multi_extension` version-ladder test
- `65d0a7ece` - `13.3-1--13.4-1` upgrade path plumbing (folds into
#8621)
- `152fdf425` - the version bump itself (#8635), cited above

### Relationship to #8719

12 of the 13 entries are shared with the 13.4.0 changelog (#8719) and
the text is
byte-identical between the two, deliberately, so the same fix reads the
same way on
both lines. The one 14.0-only entry is #8465, which was not backported
to 13.2.

Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
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.

2 participants