-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Track shard session affecting change inside the transaction #17266
Conversation
Signed-off-by: Harshit Gangal <[email protected]>
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
|
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17266 +/- ##
==========================================
+ Coverage 67.39% 67.42% +0.03%
==========================================
Files 1570 1574 +4
Lines 252917 253313 +396
==========================================
+ Hits 170446 170795 +349
- Misses 82471 82518 +47 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
go/vt/vtgate/scatter_conn.go
Outdated
@@ -894,17 +901,24 @@ type shardActionInfo struct { | |||
actionNeeded actionNeeded | |||
reservedID, transactionID int64 | |||
alias *topodatapb.TabletAlias | |||
ignoreOldSession bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite get why this ignoreOldSession
field is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for reserved connection, when they are rebuild, the old session instance held should be ignored
and a new shard session should append
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we continue to use the reserveid and transaction id fields like we were using before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Harshit Gangal <[email protected]>
Description
This PR adds tracking on per shard transaction on VTGate Session to know if DMLs caused any modification in the underlying database.
This will be used to restrict the participants of cross shard transactions ignoring non-modified shards from the list.
As this is part of VTGate Session, it cannot be used in same release as it could potentially break VTGate upgrade
Related Issue(s)
Checklist