Fix large sync sessions by reusing the same session#2036
Merged
penberg merged 2 commits intotursodatabase:mainfrom Apr 22, 2025
Merged
Fix large sync sessions by reusing the same session#2036penberg merged 2 commits intotursodatabase:mainfrom
penberg merged 2 commits intotursodatabase:mainfrom
Conversation
Currently, we have a bug where each `push` request creates a new sync session in the server. Each push request can send up to 128 frames. However, if a transaction spans more than 128 frames, then it needs to reuse the same sync session. Otherwise, the first push will succeed, but the rest of the pushes will fail due to the write lock held by the previous session. This patch reuses the sync session. The way we do this is by using batons. If the server sends a baton, then we pass it back. Reusing the same baton ensures that we are using the same sync session.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug by reusing the same sync session during consecutive push requests through the introduction of baton handling. The key changes include:
- Updated the push_frames API to accept an optional baton parameter and return a PushFramesResult struct.
- Adjusted test cases in libsql/src/sync/test.rs to align with the new return type.
- Modified the sync session logic in libsql/src/sync.rs to reuse the baton for subsequent push requests.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libsql/src/sync/test.rs | Updated tests to provide the baton parameter and assert on max_frame_no. |
| libsql/src/sync.rs | Revised the push_frames API and related logic to support baton reuse. |
Comments suppressed due to low confidence (2)
libsql/src/sync/test.rs:33
- Consider adding tests that simulate scenarios where the server returns a defined baton to ensure that baton reuse works correctly.
assert_eq!(durable_frame.max_frame_no, 0); // First frame should return max_frame_no = 0
libsql/src/sync.rs:200
- Consider adding an inline comment explaining the purpose of appending the baton to the URI, which reinforces the session reuse mechanism.
if let Some(ref baton) = baton { uri.push_str(&format!("/{}", baton)); }
Collaborator
Member
Author
|
@penberg indeed it is same, I wasn't aware that you had a patch already |
Merged
via the queue into
tursodatabase:main
with commit Apr 22, 2025
3d149d6
18 of 19 checks passed
This file contains hidden or 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
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.
Currently, we have a bug where each
pushrequest creates a new sync session in the server. Each push request can send up to 128 frames.However, if a transaction spans more than 128 frames, then it needs to reuse the same sync session. Otherwise, the first push will succeed, but the rest of the pushes will fail due to the write lock held by the previous session.
This patch reuses the sync session. The way we do this is by using batons. If the server sends a baton, then we pass it back. Reusing the same baton ensures that we are using the same sync session.