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

Cache: update add_chunk to use ChunkIndex.add #925

Merged
merged 2 commits into from
Apr 16, 2016

Conversation

enkore
Copy link
Contributor

@enkore enkore commented Apr 16, 2016

Also fixes a small cosmetic bug (would only affect recreate) where with add_chunk(overwrite=True) chunks that aren't unique would count towards the unique/deduplicated size.

@codecov-io
Copy link

Current coverage is 84.59%

Merging #925 into master will not affect coverage as of 5b36651

@@            master    #925   diff @@
======================================
  Files           14      14       
  Stmts         4778    4778       
  Branches       868     868       
  Methods          0       0       
======================================
  Hit           4042    4042       
  Partial        213     213       
  Missed         523     523       

Review entire Coverage Diff as of 5b36651


Uncovered Suggestions

  1. +0.32% via borg/helpers.py#897...911
  2. +0.25% via borg/upgrader.py#316...327
  3. +0.23% via borg/archiver.py#806...816
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@ThomasWaldmann
Copy link
Member

did you check all callers of ChunkIndex.add whether they want / don't want to update size/csize?

@enkore
Copy link
Contributor Author

enkore commented Apr 16, 2016

The only direct user of this is the Cache. In add_chunk that's what we want, in the index building it's not relevant: In which order the archive indices are merged is dictionary order thus random, hence, if there was any inconsistent csize data in the archives, the effect is random, independent of which side merge/add take the csize from.

size is only different for either corruption / logic bugs or hash collisions.

@ThomasWaldmann ThomasWaldmann merged commit 77966c7 into borgbackup:master Apr 16, 2016
@enkore enkore deleted the fix/hashindexadd branch April 18, 2016 12:26
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.

3 participants