Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Updates now retry on failure #345

Merged
merged 3 commits into from
Apr 11, 2019
Merged

Updates now retry on failure #345

merged 3 commits into from
Apr 11, 2019

Conversation

tli2
Copy link
Contributor

@tli2 tli2 commented Apr 11, 2019

Updates were too conservative from before, and immediately return false when compare-and-swap fail on the version chain. The failure could be an innocuous one from the GC. In this PR we changed updates to retry on a CAS failure and only abort after seeing an actual conflict.

@tli2 tli2 added ready-for-review This PR passes all checks and is ready to be reviewed. Mark PRs with this. performance Performance related issues or changes. labels Apr 11, 2019
@tli2 tli2 requested a review from mbutrovich April 11, 2019 19:27
@tli2 tli2 changed the title Update now retries on failure Updates now retry on failure Apr 11, 2019
@codecov
Copy link

codecov bot commented Apr 11, 2019

Codecov Report

Merging #345 into master will increase coverage by 0.08%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #345      +/-   ##
==========================================
+ Coverage   89.44%   89.52%   +0.08%     
==========================================
  Files         117      117              
  Lines        4376     4374       -2     
==========================================
+ Hits         3914     3916       +2     
+ Misses        462      458       -4
Impacted Files Coverage Δ
src/storage/data_table.cpp 97.9% <81.25%> (+0.66%) ⬆️
src/transaction/transaction_manager.cpp 94.87% <0%> (+0.85%) ⬆️
src/storage/garbage_collector.cpp 91.3% <0%> (+1.08%) ⬆️
src/network/connection_handle.cpp 52.94% <0%> (+1.96%) ⬆️

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 8d7a52f...965161e. Read the comment docs.

Copy link
Contributor

@mbutrovich mbutrovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix.

@mbutrovich mbutrovich merged commit ae49a9c into cmu-db:master Apr 11, 2019
@mbutrovich mbutrovich added ready-to-merge This PR is ready to be merged. Mark PRs with this. and removed ready-for-review This PR passes all checks and is ready to be reviewed. Mark PRs with this. labels Apr 11, 2019
@tli2 tli2 deleted the update-fix branch April 11, 2019 20:52
@tli2 tli2 restored the update-fix branch April 13, 2019 01:44
@tli2 tli2 deleted the update-fix branch April 14, 2019 19:00
@tli2 tli2 restored the update-fix branch April 17, 2019 21:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
performance Performance related issues or changes. ready-to-merge This PR is ready to be merged. Mark PRs with this.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants