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 SyncV2 protocol #5513

Merged
merged 17 commits into from
Feb 5, 2025
Merged

Add SyncV2 protocol #5513

merged 17 commits into from
Feb 5, 2025

Conversation

ChrisPenner
Copy link
Contributor

@ChrisPenner ChrisPenner commented Dec 19, 2024

Overview

Adds experimental (and hidden) pull.v2 command which uses Share's SyncV2 protocol.

Currently syncv2 is active on both staging and production :)

See caveats in Loose Ends below;
Usage:

$ UNISON_SHARE_HOST=https://staging.api.unison-lang.org stack exec unison -- -C empty
scratch/main> pull.v2 @unison/base/main

  Processed 24858 entities...

  Saved 24858 / 24858 entities...

  ✅

  Successfully pulled into scratch/main, which was empty.

scratch/main>

Implementation notes

  • Adds pull.v2 command which streams entities from Share rather than the current iterative deepening approach
  • Adds prepared statement caching to SQLite transactions, a test on staging showed an improvement from 20s -> 15s in total sync time for @unison/base/main when statement caching was enabled;
    • TODO: Check whether it's actually the statement caching or just the SQLite tuning that makes a difference here.; Yes it looks like statement caching does make a significat difference (14.5s -> 10s for a unison/base sync)
  • Leaves pull unaffected.

Test coverage

I've tested it on staging and everything seems to be working, we'll probably get it running in Nimbus CI or something soon.

There are already some automated tests around the serialization format (from the sync-file PR)

Loose ends

There are several progressive enhancements I'm still working on, none of which should be breaking changes, but best to keep this command experimental for a short while :)

  • The current version doesn't handle incremental pulls, e.g. if you pull a branch just to get most-recent changes it'll sync the entire branch's history again (obviously not ideal, I've got a fix in progress)
  • Share doesn't yet sort entities in dependency-first order, meaning UCM needs to do an in-memory sort, which slows things down and unfortunately means we need to keep ALL entities in memory during syncing 😬 ; working on a fix for this too :)

@ChrisPenner ChrisPenner force-pushed the syncv2/experiments branch 2 times, most recently from 8a69c3f to 33c5981 Compare January 15, 2025 00:31
@ChrisPenner ChrisPenner force-pushed the syncv2/experiments branch 2 times, most recently from 3acb723 to aacd13a Compare January 31, 2025 22:21
@ChrisPenner ChrisPenner changed the title [Experimental] Add SyncV2 protocol Add SyncV2 protocol Feb 4, 2025
@ChrisPenner ChrisPenner marked this pull request as ready for review February 5, 2025 17:51
@ChrisPenner ChrisPenner mentioned this pull request Feb 5, 2025
1 task
@ChrisPenner ChrisPenner changed the base branch from trunk to cp/embed-instructions February 5, 2025 18:20
@ChrisPenner ChrisPenner requested a review from a team as a code owner February 5, 2025 18:20
@ChrisPenner ChrisPenner changed the base branch from cp/embed-instructions to trunk February 5, 2025 18:21
@ChrisPenner ChrisPenner removed the request for review from a team February 5, 2025 18:21
@ChrisPenner ChrisPenner self-assigned this Feb 5, 2025
@aryairani aryairani merged commit 5282521 into trunk Feb 5, 2025
32 checks passed
@aryairani aryairani deleted the syncv2/experiments branch February 5, 2025 18:24
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