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

database: change insert to upsert in concurrency tests #55

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Conversation

psarna
Copy link
Collaborator

@psarna psarna commented Jun 13, 2023

Using insert() was a violation of our API, kind of, because inserts are not expected to be called twice on the same id. Instead, update or upsert should delete the version first, and that's what's done in this patch.

At the same time, write-write conflict detection needed to be implemented, because we started hitting it with rollback().

Finally, garbage collection is modified to actually work and garbage-collect row versions. Without it, the number of tracked row versions very quickly goes out of hand.

Using insert() was a violation of our API, kind of, because
inserts are not expected to be called twice on the same id.
Instead, update or upsert should delete the version first,
and that's what's done in this patch.

At the same time, write-write conflict detection needed to be
implemented, because we started hitting it with rollback().

Finally, garbage collection is modified to actually work
and garbage-collect row versions. Without it, the number of tracked
row versions very quickly goes out of hand.
@psarna psarna requested a review from penberg June 13, 2023 13:54
@penberg penberg merged commit ec64508 into main Jun 13, 2023
1 check failed
@penberg penberg deleted the upsert_tests branch June 13, 2023 13:55
psarna added a commit that referenced this pull request Jun 13, 2023
Fixes #55 - it was the code that should have been there
in the first place, but I forgot to `git add`...
@psarna
Copy link
Collaborator Author

psarna commented Jun 13, 2023

I forgot to git add some very crucial code to this PR... pushed as d7c59bd

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.

2 participants