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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only materialize immutable files once per process #18600

Merged
merged 4 commits into from Mar 29, 2023

Conversation

thejcannon
Copy link
Member

@thejcannon thejcannon commented Mar 27, 2023

This PR removes TempImmutableLargeFile in favor of having the caller provide a writer function which writes to the tokio::fs::File, and we wrap the writing in the necessary setup/teardown code.

This has a few benefits:

  • We can use a OnceCell to ensure the file is only trying to materialized by one thread
  • We can cleanup on error now (whoops that was embarrassing)
  • Eliminates a heisenbug I was seeing where we'd fail to hardlink with "No such file or directory".
    • I still don't understand the heisenbug, and I'm still worried we'd see it cross-process 馃槩

All in all, big win

@thejcannon thejcannon merged commit c398899 into pantsbuild:main Mar 29, 2023
17 checks passed
@thejcannon thejcannon deleted the bettertemp branch March 29, 2023 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants