Skip to content
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 transaction API for Azure SQL server #1516

Merged

Conversation

shubham1172
Copy link
Member

@shubham1172 shubham1172 commented Feb 21, 2022

Signed-off-by: Shubham Sharma [email protected]

Description

This PR introduces a logic for only keeping the latest operations per key in a transaction. This is similar to what was proposed in #1209 (comment). For example, say the original transaction is:

upsert k1 v1  # redundant, gets deleted later
delete k1
delete k2     # redundant, gets set to v2 later
upsert k2 v2

Since #1517 was a prerequisite, I have merged the changes with this one. It ignores row mismatch error while using bulk delete in transactions.

Issue reference

Please reference the issue this PR will close: #1505, #1517

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

@shubham1172 shubham1172 requested review from a team as code owners February 21, 2022 07:42
@shubham1172 shubham1172 changed the title [WIP] Fix transaction ordering for Azure SQL server [WIP] Fix transaction API for Azure SQL server Feb 21, 2022
@shubham1172
Copy link
Member Author

shubham1172 commented Feb 21, 2022

Test from #1496
image

For reference with master:
image

@shubham1172 shubham1172 changed the title [WIP] Fix transaction API for Azure SQL server Fix transaction API for Azure SQL server Feb 21, 2022
Signed-off-by: Shubham Sharma <[email protected]>
@shubham1172 shubham1172 force-pushed the shubham1172/fix-sql-server-transaction-api branch from 1548d09 to acde0fb Compare February 21, 2022 10:50
@yaron2
Copy link
Member

yaron2 commented Feb 21, 2022

This is a needed fix, thanks for the PR.

@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #1516 (e6d3b90) into master (a03d7a6) will increase coverage by 0.07%.
The diff coverage is 41.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1516      +/-   ##
==========================================
+ Coverage   35.29%   35.37%   +0.07%     
==========================================
  Files         157      158       +1     
  Lines       14255    14336      +81     
==========================================
+ Hits         5031     5071      +40     
- Misses       8700     8730      +30     
- Partials      524      535      +11     
Impacted Files Coverage Δ
configuration/redis/redis.go 33.55% <0.00%> (+1.27%) ⬆️
state/errors.go 70.58% <0.00%> (-21.72%) ⬇️
state/sqlserver/sqlserver.go 41.39% <0.00%> (-1.42%) ⬇️
state/jetstream/jetstream.go 50.81% <50.81%> (ø)
pubsub/rabbitmq/rabbitmq.go 59.68% <57.14%> (-0.63%) ⬇️
pubsub/rabbitmq/metadata.go 93.54% <100.00%> (+0.82%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70a86e6...e6d3b90. Read the comment docs.

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.

Azure SQL Server state store changes order of transaction
4 participants