-
Notifications
You must be signed in to change notification settings - Fork 800
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
Fix checksum validation for SQL implementation #5790
Conversation
bf45163
to
a26b329
Compare
Codecov Report
Additional details and impacted files
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
7ffd197
to
9879c0f
Compare
b3cd4eb
to
40810ba
Compare
What changed?
Add a check for the SQL implementation of GetWorkflowExecution operation to exclude false positive checksum validation failure cases.
Why?
To make sure the checksum validation result is true, the data we read from GetWorkflowExecution operation are from a consistent view. In the NoSQL implementation, the operation is a single read, so the data is from a consistent view. However, in the SQL implementation, the operation is multiple reads from different table. If there is a concurrent update, the data we read isn't from a consistent view and the checksum validation could fail. Normally, we don't have concurrent updates with reads. But when the shard ownership changed, it might not be the case.
How did you test it?
unit tests
Potential risks
Release notes
Documentation Changes