vtgate: restrict 2PC commit path to only modified shards #19256
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Samriddha9619
requested review from
frouioui,
harshit-gangal and
systay
as code owners
vitess-bot
bot
added
NeedsWebsiteDocsUpdate
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Samriddha9619
force-pushed
the
restrict-2pc-modified-shards
branch
from
e4706d9 to
e450493
Compare
Contributor
|
📝 Documentation updates detected! New suggestion: Document TwoPC performance optimizations |
Comment on lines
265
to
292
| var modifiedShards []*vtgatepb.Session_ShardSession | ||
| var readOnlyShards []*vtgatepb.Session_ShardSession | ||
|
|
||
| for _, s := range session.ShardSessions { | ||
| if s.RowsAffected { | ||
| modifiedShards = append(modifiedShards, s) | ||
| } else { | ||
| readOnlyShards = append(readOnlyShards, s) | ||
| } | ||
| } | ||
|
|
||
| for _, s := range readOnlyShards { | ||
| _ = txc.commitShard(ctx, s, session.GetLogger()) | ||
| } | ||
|
|
||
| // If the number of participants is one or less, then it's a normal commit. | ||
| if len(session.ShardSessions) <= 1 { | ||
| if len(modifiedShards) <= 1 { | ||
| originalShards := session.ShardSessions | ||
| session.ShardSessions = modifiedShards | ||
| defer func() { session.ShardSessions = originalShards }() | ||
|
|
||
| return txc.commitNormal(ctx, session) | ||
| } | ||
|
|
||
| originalShards := session.ShardSessions | ||
| session.ShardSessions = modifiedShards | ||
| defer func() { session.ShardSessions = originalShards }() | ||
|
|
Member
There was a problem hiding this comment.
The idea behind this change is solid, but I'm not sure the implementation is going in the right direction.
Modifying ShardSessions and having the changes be reverted through a defer is confusing, as it's no longer clear what the contents of ShardSessions will be at different points of the code..
Author
There was a problem hiding this comment.
@arthurschreiber Thanks for the review.
In the updated PR I have refactored commit2PC to filter the shards into local slices and pass them explicitly to the transaction logic.
Does this approach look better to you? Happy to adjust if I overlooked anything else.
Samriddha9619
force-pushed
the
restrict-2pc-modified-shards
branch
from
e450493 to
963147e
Compare
Signed-off-by: Samriddha9619 <sumitkumartripathi0@gmail.com>
Samriddha9619
force-pushed
the
restrict-2pc-modified-shards
branch
from
963147e to
672fb3b
Compare
harshit-gangal
approved these changes
Feb 3, 2026
Member
harshit-gangal
left a comment
harshit-gangal
left a comment
There was a problem hiding this comment.
changes looks good to me.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR optimizes the Two-Phase Commit (2PC) path in VTGate by ensuring that only shards that have been modified (tracked via the
RowsAffectedflag) participate in the atomic commit flow.Changes
Separate shards by modification status: The
commit2PCfunction now separatesShardSessionsintomodifiedShardsandreadOnlyShardsbased on theRowsAffectedflag.Direct commit for read-only shards: Read-only shards are committed directly using
commitShard(), bypassing 2PC. This releases their resources immediately without the overhead of prepare/commit phases.Single-shard optimization: If only one shard was modified, we use normal commit instead of 2PC. This is a significant performance improvement as it avoids 2PC entirely.
2PC only for multiple modified shards: The full 2PC flow (CreateTransaction → Prepare → StartCommit → CommitPrepared → Conclude) is only executed when multiple shards have been modified.
Impact
Related Issue(s)
Fixes #18054
Checklist