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

Add penultimate key #1355

Merged
merged 3 commits into from
Mar 4, 2024
Merged

Add penultimate key #1355

merged 3 commits into from
Mar 4, 2024

Conversation

willdealtry
Copy link
Collaborator

Add the last undeleted key to the version ref key so that the penultimate index can be read even when the most recent version key has not yet been replicated due to AWS out-of-order replication

@willdealtry willdealtry changed the title WIP: Add penultimate key Add penultimate key Feb 22, 2024
@willdealtry willdealtry marked this pull request as ready for review February 22, 2024 12:25
@@ -20,7 +20,6 @@ struct ReadOptions {
std::optional<bool> set_tz_;
std::optional<bool> optimise_string_memory_;
std::optional<bool> batch_throw_on_error_;
std::optional<bool> read_previous_on_failure_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@poodlewars has a PR out that removes this as well, just to be aware for whoever's merging second

@willdealtry willdealtry force-pushed the add_penultimate_key branch 5 times, most recently from a4a5192 to 2093a05 Compare February 27, 2024 13:58
@willdealtry willdealtry merged commit 116af13 into master Mar 4, 2024
110 checks passed
@willdealtry willdealtry deleted the add_penultimate_key branch March 4, 2024 12:17
@DrNickClarke
Copy link
Collaborator

I ran tests that reliably created ref key errors when run against a replicated AWS bucket, due to out of order replication.

With the change from this those errors vanished, so it has solved the problem.

poodlewars added a commit that referenced this pull request May 15, 2024
)

#### What does this implement or fix?

We should only write the version ref key once when we write with
`prune_previous_versions=True`. Currently we are writing it twice - once
after we write the tombstone all and once when we write the new version.
This means that there is a period of time where the symbol is
unreadable.

This was fixed a while ago with PR #1104 but regressed with PR #1355.
poodlewars added a commit that referenced this pull request May 15, 2024
)

#### What does this implement or fix?

We should only write the version ref key once when we write with
`prune_previous_versions=True`. Currently we are writing it twice - once
after we write the tombstone all and once when we write the new version.
This means that there is a period of time where the symbol is
unreadable.

This was fixed a while ago with PR #1104 but regressed with PR #1355.
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.

3 participants