Fix lost updates with long running transactions #29
Conversation
This file contains 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
The original c extension we aim to be a drop-in replacements has a check to avoid insterting multiple history rows in the same transaction (see arkhipov/temporal_tables@b563fb3) We kept this check in our port, but we introduced a bug in which updated and deletes from all currently running transactions may also be discarded, because we check `xmin` against `txid_current_snapshot`. I fix it as suggested by @kjs222 by checking only against `txid_current`.
|
Hey @paolochiodi thanks for your work on this. The ability to use Temporal Table patterns in environments like AWS is awesome. Just wondering if there are any updates for this PR or reasons why it could not be merged? Following the discussion from #27 it seems like this is potentially an important fix. |
|
Should this PR be closed since the fix is applied on master? 34994fe I was confused by the open PR. |
|
I don't know, considering that #27 is still open. @paolochiodi ? |
|
Wait maybe I was confused and looking at the history of this branch somehow. I don't think this is merged. |
|
@brendanlong I think the commit you see is in the draft PR linked to that issue, which is this page :) |
ruberecken
approved these changes
Dec 21, 2022
simoneb
approved these changes
Jan 3, 2023
This was referenced Jan 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The original c extension we aim to be a drop-in replacements
has a check to avoid insterting multiple history rows
in the same transaction
(see arkhipov/temporal_tables@b563fb3)
We kept this check in our port, but we introduced a bug in which
updated and deletes from all currently running transactions
may also be discarded, because we check
xminagainsttxid_current_snapshot.I fix it as suggested by @kjs222 on #27 by checking it only against
txid_current.