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

irmin-pack: add a migration function to the V2 format #1070

Merged
merged 17 commits into from Aug 25, 2020

Conversation

craigfe
Copy link
Member

@craigfe craigfe commented Aug 21, 2020

This contains a subset of the changes from #1015, but uses a different implementation strategy: rather than iterating over all of the objects in the V1 store and adding them individually to the V2 store, we delegate migration to the IO instances (which just shift everything along the appropriate number of bytes). This avoids some unnecessary work.

Other differences:

  • due to the new implementation, the migration does not touch the index at all – the Index format has not changed (yet... CC @pascutto);
  • the temporary stores are created inside the existing one, to ensure that it's on the same device;
  • upgrading a store poisons all instance caches with unlinked file descriptors. Rather than trying to detect this dynamically, I've gone with invalidating cache entries. This means that having open instances during migration is undefined behaviour.

@craigfe
Copy link
Member Author

craigfe commented Aug 21, 2020

Aside: if we suspect this migration is very disruptive to users, perhaps we should consider adding some amount of blank header space to the IO files in case a future format change can quickly take advantage of those.

@craigfe
Copy link
Member Author

craigfe commented Aug 21, 2020

Needing to extract Filename.quote_command here is quite painful. Perhaps we should: (a) add a dependency (b) use Dune more cleverly (c) spend more time thinking about what quoting needs to be done on Windows.

src/irmin-pack/IO.ml Outdated Show resolved Hide resolved
src/irmin-pack/pack.ml Outdated Show resolved Hide resolved
src/irmin-pack/IO.ml Outdated Show resolved Hide resolved
@craigfe craigfe force-pushed the v2-format-migration branch 2 times, most recently from fdfa75f to 8453d10 Compare August 24, 2020 12:02
@samoht
Copy link
Member

samoht commented Aug 24, 2020

The progress bar tests seem broken

@craigfe
Copy link
Member Author

craigfe commented Aug 24, 2020

Indeed... that's what I get for using this PR as a way to get code onto comanche...

Fixed.

@samoht
Copy link
Member

samoht commented Aug 25, 2020

Could you rebase on top of master now that #1072 is merged?

@craigfe
Copy link
Member Author

craigfe commented Aug 25, 2020

@samoht: Done.

@samoht
Copy link
Member

samoht commented Aug 25, 2020

Thanks! The new code looks indeed cleaner and simpler. Merging now.

@samoht samoht merged commit 27238ec into mirage:master Aug 25, 2020
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