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

remove empty shadowed_segments lists, fixes #5275 #5614

Conversation

ThomasWaldmann
Copy link
Member

also:

  • add test for removed empty shadowed_segments list
  • add some comments
  • add repo_dump test debug tool

@ThomasWaldmann
Copy link
Member Author

@enkore can you please try & review?

@timmc
Copy link

timmc commented Jan 17, 2021

I notice that Repository.put doesn't add to shadow_index -- is that intentional? Not sure if there's an impact on this PR.

@ThomasWaldmann ThomasWaldmann force-pushed the remove-empty-shadowed-segments-list-master branch from 6f184df to c3a3f34 Compare January 18, 2021 19:35
@codecov-io
Copy link

codecov-io commented Jan 18, 2021

Codecov Report

Merging #5614 (6f00b02) into master (9c988ee) will decrease coverage by 0.26%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5614      +/-   ##
==========================================
- Coverage   82.82%   82.55%   -0.27%     
==========================================
  Files          38       38              
  Lines       10154    10062      -92     
  Branches     1682     1674       -8     
==========================================
- Hits         8410     8307     -103     
- Misses       1238     1246       +8     
- Partials      506      509       +3     
Impacted Files Coverage Δ
src/borg/repository.py 83.93% <50.00%> (-0.10%) ⬇️
src/borg/fuse_impl.py 30.76% <0.00%> (-23.08%) ⬇️
src/borg/helpers/misc.py 79.04% <0.00%> (-1.20%) ⬇️
src/borg/upgrader.py 60.94% <0.00%> (-1.13%) ⬇️
src/borg/remote.py 79.01% <0.00%> (-0.54%) ⬇️
src/borg/helpers/process.py 65.00% <0.00%> (-0.44%) ⬇️
src/borg/archiver.py 79.43% <0.00%> (-0.36%) ⬇️
src/borg/archive.py 80.17% <0.00%> (-0.27%) ⬇️
src/borg/logger.py 70.33% <0.00%> (-0.25%) ⬇️
src/borg/crypto/key.py 86.38% <0.00%> (-0.22%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c988ee...6f00b02. Read the comment docs.

@ThomasWaldmann
Copy link
Member Author

rebased on current master.

@ThomasWaldmann
Copy link
Member Author

ThomasWaldmann commented Jan 18, 2021

@timmc good catch!

we will have to check whether there is a bug or that has to be like that, not sure yet.

@ThomasWaldmann
Copy link
Member Author

ThomasWaldmann commented Jan 18, 2021

@timmc @enkore see discussion above and #5636

@ThomasWaldmann ThomasWaldmann force-pushed the remove-empty-shadowed-segments-list-master branch from c7f0aac to c3a3f34 Compare January 18, 2021 20:51
@ThomasWaldmann ThomasWaldmann added this to the hydrogen-b2 milestone Jan 24, 2021
@ThomasWaldmann
Copy link
Member Author

I'll merge this soon, but it would be good if there was some more independent review.

also:
- add test for removed empty shadowed_segments list
- add some comments
- add repo_dump test debug tool
@ThomasWaldmann ThomasWaldmann force-pushed the remove-empty-shadowed-segments-list-master branch from c3a3f34 to 6f00b02 Compare January 29, 2021 14:45
@ThomasWaldmann ThomasWaldmann merged commit e3d8b7c into borgbackup:master Jan 29, 2021
@ThomasWaldmann ThomasWaldmann deleted the remove-empty-shadowed-segments-list-master branch January 29, 2021 15:33
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this pull request Feb 15, 2021
also:
- add test for removed empty shadowed_segments list
- add some comments
- add repo_dump test debug tool

note: this is the backport of borgbackup#5614 to 1.1-maint - it needed some adaptions to fit into 1.1-maint:

1.1-maint always compacts when committing and does not persist shadow_index, so the tests are a bit different
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