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

libsql: add read your writes support #790

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

LucioFranco
Copy link
Contributor

@LucioFranco LucioFranco commented Dec 14, 2023

Fixes #623

@@ -191,6 +191,10 @@ impl RemoteConnection {
.into();
}

if let Some(replicator) = writer.replicator() {
replicator.sync_oneshot().await?;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify - this is the important bit that gives us read-before-wrote, right? Combined with the fact that writer.replicator is only engaged if we passed read_before_write == true before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, @LucioFranco, res returned from execute_program contains the latest frame number after the program got executed, right? So perhaps it makes sense to make sync_oneshot take a frame_no parameter, and sync just to this specific frame, instead of waiting until we catch up with the primary (so instead of returning from sync_oneshot when replica_index >= primary_index, we'd return when replica_index >= that_index_we_just_received_in_the_response_above)? If I understand correctly, it could make read-your-writes more leightweight, because we'd only wait for our write, not potentially somebody else's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, lets do this in a follow up 👍

@LucioFranco LucioFranco force-pushed the lucio/stream-embedded-replica branch from 53db85d to 6405199 Compare December 18, 2023 15:15
@LucioFranco LucioFranco force-pushed the lucio/stream-embedded-replica branch from 6405199 to cffd595 Compare December 18, 2023 19:39
@LucioFranco LucioFranco added this pull request to the merge queue Dec 18, 2023
Merged via the queue into main with commit a42bbcc Dec 18, 2023
@LucioFranco LucioFranco deleted the lucio/stream-embedded-replica branch December 18, 2023 21:00
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.

Make write imply sync by default
2 participants