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

Only extract declared outputs from firecracker workspaces #7959

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

bduffany
Copy link
Member

@bduffany bduffany commented Nov 26, 2024

This should significantly cut down on IO due to extracting lots of files from the guest workspace drive that aren't needed, and also unlinking them afterwards.

Depends on #7957 so that we don't have to guard against path traversals in this new code (since we're passing the user-provided output_paths to this new func).

@bduffany bduffany force-pushed the debugfs-optimize branch 10 times, most recently from 32da903 to 49c3ba4 Compare November 26, 2024 03:28
@bduffany bduffany changed the title Extract only output paths from firecracker workspaces Extract only declared outputs from firecracker workspaces Nov 26, 2024
@bduffany bduffany force-pushed the debugfs-optimize branch 3 times, most recently from ac87359 to 9266235 Compare November 26, 2024 03:39
@bduffany bduffany marked this pull request as ready for review November 26, 2024 03:39
@bduffany bduffany force-pushed the debugfs-optimize branch 3 times, most recently from 9698bec to 1d5a0ad Compare November 26, 2024 03:53
// TODO: declare this list as a constant somewhere?
paths := []string{
".BUILDBUDDY_DO_NOT_RECYCLE",
".BUILDBUDDY_INVALIDATE_SNAPSHOT",
Copy link
Member Author

Choose a reason for hiding this comment

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

@maggie-lou FYI I preemptively added the new file here from your in-flight PR. After our PRs are both merged I'm thinking of maybe adding a list to container.go with these special file names

@bduffany bduffany changed the title Extract only declared outputs from firecracker workspaces Only extract declared outputs from firecracker workspaces Nov 27, 2024
@bduffany bduffany merged commit baac073 into master Dec 6, 2024
15 checks passed
@bduffany bduffany deleted the debugfs-optimize branch December 6, 2024 17:48
bduffany added a commit that referenced this pull request Dec 16, 2024
We aren't actively using this code. The `debugfs` codepath is probably
"fast enough" since we updated it to only copy output paths in
#7959, and also this
code is likely to be supplanted by FUSE anyway. If we do want to try
this code again in the future, we can always bring it back, but should
test thoroughly in dev first because we hit some issues with the
loopback device approach last time.
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