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

feat: add history rebuild #1575

Merged
merged 4 commits into from Jan 16, 2024
Merged

feat: add history rebuild #1575

merged 4 commits into from Jan 16, 2024

Conversation

ellie
Copy link
Member

@ellie ellie commented Jan 15, 2024

Previously we had all parts working independently, but history didn't actually end up usable when synced via record store

Now

  1. Add atuin store rebuild history. This should be used when we wish to trigger a total rebuild of the history database from the record store. You can actually just delete history.db if you fancy and the state all comes back OK. Deletes no longer rely on the deleted_at field, and are actually removed from history.db
  2. Make the record download step return all IDs that it has downloaded
  3. Add incremental_build, which takes all downloaded IDs and processes them. If they're history records, add them to the history store

There are many, many places that this can be optimised. At the moment the full rebuild is a bit slow, though this shouldn't be used often. The incremental build seems to perform pretty quickly in my testing so far 🙏

This is fine as opt-in for the next release, but will require a bunch of testing

Opt-in with

[sync]
records = true

in config.

Also note that deletes don't function with the record store, but will follow up with that shortly. Just need to push a HistoryRecord::Delete properly. Works fine when you import the old history db tho

This adds a function that will

1. List all history from the store
2. Segment by create/delete
3. Insert all creates into the database
4. Delete all deleted

This replaces the old history sync.

Presently it's incomplete. There is no incremental rebuild, it can only
do the entire thing at once.

This is ran by `atuin store rebuild history`
Copy link

vercel bot commented Jan 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
atuin-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2024 11:18am

@conradludgate
Copy link
Collaborator

That's pretty speedy. I guess this is light work for sqlite. I wonder if a hdd would be considerably slower here

@ellie
Copy link
Member Author

ellie commented Jan 16, 2024

I think the bottleneck is more decrypting every single entry vs loading a few mb from SQLite. Will actually benchmark it properly at some point though

Slowness isn't a huge problem with a full rebuild right now. It's not intended to run often, only either when importing a bunch or for debugging something

Incremental builds almost there 🙏

@ellie
Copy link
Member Author

ellie commented Jan 16, 2024

turns out doing sync via the new method + incremental_build is

  1. fast as fuck
  2. uses much less network

...and I haven't spent any time optimising it, at all

@conradludgate
Copy link
Collaborator

Who would have thought the sync structure we designed to be good for incremental sync is good for incremental sync

@conradludgate
Copy link
Collaborator

I think the bottleneck is more decrypting every single entry

We could probably make use of rayon at some point to multi-thread it. but certainly not a priority

@ellie
Copy link
Member Author

ellie commented Jan 16, 2024

yeah for sure. there's a whole bunch of places where it can be sped up in the future

@ellie ellie marked this pull request as ready for review January 16, 2024 11:14
@ellie ellie merged commit a2578c4 into main Jan 16, 2024
10 checks passed
@ellie ellie deleted the ellie/history-builder branch January 16, 2024 11:25
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.

None yet

2 participants