Skip to content

Conversation

@ckyrouac
Copy link
Collaborator

@ckyrouac ckyrouac commented Sep 2, 2025

This is to avoid creating (10s of) thousands of links to a single file which will cause the rechunker to fail after passing the filesystem's link limit. This will create a duplicate xattrs file to link to for each chunk of 256 links.

Assisted-by: Claude Code

Fixes coreos/rpm-ostree#5479

@bootc-bot bootc-bot bot requested a review from cgwalters September 2, 2025 19:32
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to handle a large number of links to the same xattrs object by creating duplicate xattrs files with a suffix, which is a good approach to avoid hitting filesystem link limits. The implementation adds tracking for link counts and creates new suffixed files when a limit is reached. The changes are mostly in append_ostree_xattrs and are accompanied by new tests.

My review identifies a potential logic bug in the new implementation that could cause issues if a suffixed xattrs file already exists when it's being created. I've suggested a refactoring that fixes this bug and improves efficiency. I also have a minor suggestion to improve test failure diagnostics. Overall, the change is in the right direction but needs a small correction.

@ckyrouac ckyrouac marked this pull request as draft September 2, 2025 21:28
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The export changes look sane to me. Can you do a before/after comparison on sizes just as a sanity check?

Honestly it could very well be that this saves space because I bet the xattr data is smaller than the file path in the hardlink...

This removes the code from export that links duplicate xattrs to
a single xattrs file. This is to avoid creating too many (over 65,000)
hardlinks to a single file. This might cause some image sizes to
increase, however compression should limit the size increase.

This is accomplished by creating the file-xattrs-link file directly,
instead of creating it as a link. This is to support backwards
compatibility in import.rs

Assisted-by: Claude Code
Signed-off-by: ckyrouac <[email protected]>
@ckyrouac ckyrouac marked this pull request as ready for review September 4, 2025 13:06
@bootc-bot bootc-bot bot requested a review from cgwalters September 4, 2025 13:06
@cgwalters cgwalters changed the title rechunker: Duplicate xattrs when more than 256 links tar: Create a new xattrs file for each checksum Sep 4, 2025
@cgwalters cgwalters merged commit 276018f into bootc-dev:main Sep 4, 2025
34 of 35 checks passed
@ckyrouac
Copy link
Collaborator Author

ckyrouac commented Sep 4, 2025

Can you do a before/after comparison on sizes just as a sanity check?

The quay.io/fedora/fedora-bootc:42 image is actually a few MB smaller without links:

With links: 1917.763676 MB
Without links: 1899.812956 MB

@cgwalters
Copy link
Collaborator

Thanks for confirming that, very helpful to know.

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.

Rechunker fails with too many links

2 participants