Skip to content

De-duplicate filesystem emplace - #2396

Merged
MarcusSorealheis merged 2 commits into
TraceMachina:mainfrom
palfrey:add-hardlink-failure-log
Jun 12, 2026
Merged

De-duplicate filesystem emplace#2396
MarcusSorealheis merged 2 commits into
TraceMachina:mainfrom
palfrey:add-hardlink-failure-log

Conversation

@palfrey

@palfrey palfrey commented Jun 3, 2026

Copy link
Copy Markdown
Member

Description

We've seen intermittent hardlink failures on the buildstream runs (e.g. https://github.com/TraceMachina/nativelink/actions/runs/26881965504/job/79284041143?pr=2381) but the failure case was unclear. After some experimentation, I've been able to figure it out, both by adding logging here, and then debugging based on that.

The hardlink code first gets a file entry from the filesystem cache, then hardlinks to it. Most of the time, this succeeds, and the primary previous failure case was "your filesystem cache isn't big enough for a single build run, so it's evicting between getting and linking", which was a config fix. I added a logging helper that dumps the cache status, and established this wasn't the case for the buildstream case (was hitting ~1% of cache usage).

The problem for buildstream is that for some reason a single file (usually a Python library file in the case of buildstream) is being uploaded multiple times, and so we evict the previous instance and put a new one in. Sometimes, if we're very unlucky (~1/3rd of the time on buildstream runs right now), we're doing hardlinking in the middle of that.

This PR checks for duplicate file entries i.e. we're trying to emplace a file with identical contents to the current one, and so therefore we can throw away the new entry. This means the file is always correctly in place on the filesystem, and so the hardlink should no longer fail in the previous way.

Type of change

Please delete options that aren't relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

bazel test //...

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@vercel

vercel Bot commented Jun 3, 2026

Copy link
Copy Markdown

@palfrey is attempting to deploy a commit to the native-link-web-assets Team on Vercel.

A member of the Team first needs to authorize it.

@vercel

vercel Bot commented Jun 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nativelink Ready Ready Preview, Comment Jun 12, 2026 2:47am
nativelink-aidm Ready Ready Preview, Comment Jun 12, 2026 2:47am

Request Review

@palfrey
palfrey force-pushed the add-hardlink-failure-log branch 6 times, most recently from 86abbd3 to e1e05ad Compare June 8, 2026 10:12
@palfrey palfrey changed the title Add more logging on hardlink failure De-duplicate filesystem emplace Jun 10, 2026
@palfrey
palfrey force-pushed the add-hardlink-failure-log branch from dd61b5f to e58cc5a Compare June 10, 2026 10:17

case $nativelink_output in
*"ERROR"* )
*\"ERROR\"* )

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because the new logs now contain the word error in some file paths, which is fine. We're looking for a log message error, which has quoted ERROR. Bash case uses non-escaped quotes to handle the case for matches with spaces, which is not what we want.

@MarcusSorealheis
MarcusSorealheis enabled auto-merge (squash) June 12, 2026 02:47
@MarcusSorealheis
MarcusSorealheis merged commit 73029bb into TraceMachina:main Jun 12, 2026
36 checks passed
@palfrey
palfrey deleted the add-hardlink-failure-log branch June 12, 2026 07:22
rejuvenile pushed a commit to rejuvenile/nativelink that referenced this pull request Jul 13, 2026
Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
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