Skip to content

Fix "failed to create hardlink" error due to multiple mounts on the same device#19894

Merged
stuhood merged 2 commits intopantsbuild:mainfrom
stuhood:stuhood/hardlink-canary
Sep 22, 2023
Merged

Fix "failed to create hardlink" error due to multiple mounts on the same device#19894
stuhood merged 2 commits intopantsbuild:mainfrom
stuhood:stuhood/hardlink-canary

Conversation

@stuhood
Copy link
Copy Markdown
Member

@stuhood stuhood commented Sep 20, 2023

As described in #18757, the "check that source and dest are on same device" strategy that was introduced in #18153 to decide whether we could hardlink when materializing files was not robust when faced with the same device being mounted in multiple locations.

This change moves to a "create a canary" strategy for deciding when hardlinking between two destinations is legal. Hardlinks are canaried and memoized on a per-destination-root basis, so this strategy might actually be slightly cheaper than the previous one.

Fixes #18757.

@stuhood stuhood added needs-cherrypick [CI] category:bugfix Bug fixes for released features labels Sep 20, 2023
@stuhood stuhood added this to the 2.17.x milestone Sep 20, 2023
@stuhood stuhood force-pushed the stuhood/hardlink-canary branch from 2b80593 to 1eff84a Compare September 20, 2023 23:20
thejcannon
thejcannon reviewed Sep 21, 2023
View reviewed changes
Comment thread src/rust/engine/process_executor/src/main.rs Outdated
Comment thread src/rust/engine/fs/store/src/local.rs
huonw
huonw approved these changes Sep 21, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Nice, makes sense to try the operation that we need to work as a test 👍

Only minor questions.

Comment thread src/rust/engine/fs/store/src/lib.rs Outdated
Comment thread src/rust/engine/fs/store/src/lib.rs Outdated
Comment thread src/rust/engine/process_executor/src/main.rs Outdated
@stuhood stuhood force-pushed the stuhood/hardlink-canary branch from 7d0bac8 to 57fdc96 Compare September 21, 2023 18:07
@stuhood stuhood enabled auto-merge (squash) September 21, 2023 18:59
@stuhood
Copy link
Copy Markdown
Member Author

stuhood commented Sep 21, 2023

CI is failing due to #19901. Once that one is in I'll rebase and merge.

@stuhood stuhood force-pushed the stuhood/hardlink-canary branch from 57fdc96 to 2e653b5 Compare September 22, 2023 03:15
@stuhood stuhood merged commit d774751 into pantsbuild:main Sep 22, 2023
WorkerPants pushed a commit that referenced this pull request Sep 22, 2023
@stuhood @WorkerPants
Fix "failed to create hardlink" error due to multiple mounts on the s…
b138b96 
…ame device (#19894)

As described in #18757, the "check that source and dest are on same
device" strategy that was introduced in #18153 to decide whether we
could hardlink when materializing files was not robust when faced with
the same device being mounted in multiple locations.

This change moves to a "create a canary" strategy for deciding when
hardlinking between two destinations is legal. Hardlinks are canaried
and memoized on a per-destination-root basis, so this strategy might
actually be slightly cheaper than the previous one.

Fixes #18757.
@WorkerPants
Copy link
Copy Markdown
Member

WorkerPants commented Sep 22, 2023

I tried to automatically cherry-pick this change back to each relevant milestone, so that it is available in those older releases of Pants.

❌ 2.17.x

I was unable to cherry-pick this PR to 2.17.x, likely due to merge-conflicts.

Steps to Cherry-Pick locally

To resolve:

  1. (Ensure your git working directory is clean)
  2. Run the following script to reproduce the merge-conflicts: 
    git fetch https://github.com/pantsbuild/pants main \
  && git fetch https://github.com/pantsbuild/pants 2.17.x \
  && git checkout -b cherry-pick-19894-to-2.17.x FETCH_HEAD \
  && git cherry-pick d7747516250f2aaffc17a86d0afeb6a061fca87f
  3. Fix the merge conflicts and commit the changes
  4. Run build-support/cherry_pick/make_pr.sh "19894" "2.17.x"

Please note that I cannot re-run CI if a job fails. Please work with your PR approver(s) to re-run CI if necessary.

✔️ 2.18.x

Successfully opened #19910.

When you're done manually cherry-picking, please remove the needs-cherrypick label on this PR.

Thanks again for your contributions!

🤖 Beep Boop here's my run link

@WorkerPants WorkerPants added the auto-cherry-picking-failed [CI] Auto Cherry-Picking Failed label Sep 22, 2023
@stuhood stuhood deleted the stuhood/hardlink-canary branch September 22, 2023 14:28
stuhood added a commit to stuhood/pants that referenced this pull request Sep 22, 2023
@stuhood
Fix "failed to create hardlink" error due to multiple mounts on the s…
d1a096c 
…ame device (pantsbuild#19894)

As described in pantsbuild#18757, the "check that source and dest are on same
device" strategy that was introduced in pantsbuild#18153 to decide whether we
could hardlink when materializing files was not robust when faced with
the same device being mounted in multiple locations.

This change moves to a "create a canary" strategy for deciding when
hardlinking between two destinations is legal. Hardlinks are canaried
and memoized on a per-destination-root basis, so this strategy might
actually be slightly cheaper than the previous one.

Fixes pantsbuild#18757.
stuhood added a commit that referenced this pull request Sep 22, 2023
@stuhood
Fix "failed to create hardlink" error due to multiple mounts on the s…
c878d3b 
…ame device (#19894)

As described in #18757, the "check that source and dest are on same
device" strategy that was introduced in #18153 to decide whether we
could hardlink when materializing files was not robust when faced with
the same device being mounted in multiple locations.

This change moves to a "create a canary" strategy for deciding when
hardlinking between two destinations is legal. Hardlinks are canaried
and memoized on a per-destination-root basis, so this strategy might
actually be slightly cheaper than the previous one.

Fixes #18757.
stuhood added a commit that referenced this pull request Sep 22, 2023
@WorkerPants @stuhood
Fix "failed to create hardlink" error due to multiple mounts on the s…
a34d37b 
…ame device (Cherry-pick of #19894) (#19910)

As described in #18757, the "check that source and dest are on same
device" strategy that was introduced in #18153 to decide whether we
could hardlink when materializing files was not robust when faced with
the same device being mounted in multiple locations.

This change moves to a "create a canary" strategy for deciding when
hardlinking between two destinations is legal. Hardlinks are canaried
and memoized on a per-destination-root basis, so this strategy might
actually be slightly cheaper than the previous one.

Fixes #18757.

Co-authored-by: Stu Hood <stuhood@gmail.com>
stuhood added a commit that referenced this pull request Sep 22, 2023
@stuhood
Fix "failed to create hardlink" error due to multiple mounts on the s…
e957eb5 
…ame device (Cherry-pick of #19894) (#19914)

As described in #18757, the "check that source and dest are on same
device" strategy that was introduced in #18153 to decide whether we
could hardlink when materializing files was not robust when faced with
the same device being mounted in multiple locations.

This change moves to a "create a canary" strategy for deciding when
hardlinking between two destinations is legal. Hardlinks are canaried
and memoized on a per-destination-root basis, so this strategy might
actually be slightly cheaper than the previous one.

Fixes #18757.
@huonw huonw removed the needs-cherrypick [CI] label Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@huonw huonw huonw approved these changes

@thejcannon thejcannon thejcannon left review comments

@jriddy jriddy Awaiting requested review from jriddy

Assignees

No one assigned

Labels

auto-cherry-picking-failed [CI] Auto Cherry-Picking Failed category:bugfix Bug fixes for released features

Projects

None yet

Milestone

2.17.x

Development

Successfully merging this pull request may close these issues.

"Failed to create hardlink" due to "Invalid cross-device link (os error 18)" in 2.17.0.dev

4 participants

@stuhood @WorkerPants @huonw @thejcannon