-
Notifications
You must be signed in to change notification settings - Fork 219
fix: speed up provisioning shutdown #1507
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
Conversation
b856cee to
a7ce236
Compare
| return nil, err | ||
| } | ||
|
|
||
| if int64(n) != m.blockSize { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ok, just thinking if there is no way the last block/page can be short (it would violate other assumptions though.)
| defer tf.Close(s.config) | ||
|
|
||
| // The snapfile is required only because the FC API doesn't support passing /dev/null | ||
| snapfile := template.NewLocalFileLink(tf.CacheSnapfilePath(s.config)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the modified FC, would you prefer to have an ability to skip snapfile too/have it delivered as returned body from pause, that you can then more easily discard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this use case, yes, accepting it in body or be able to put there /dev/null would be useful. But before that, let's see if it's desired that the reboot doesn't flush. Waiting on a reply from the FC team
ValentaTomas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments that I would like to sync on, but otherwise lgtm.
Note
Introduce Sandbox.Shutdown for faster, clean VM teardown during provisioning; ensure full block writes in diff export and add cache cleanup for template cache files.
Sandbox.Shutdown(ctx)to pause VM, disable uffd, snapshot to temp files, and close resources.Shutdownin base provision phase instead ofPausesnapshot workflow; remove unused snapshot handling.block/cache.ExportToDiff, verify full block writes by checkingnagainstblockSizeand error on short write.TemplateCacheFiles.Close(config)to remove cache dir; used bySandbox.Shutdown.Written by Cursor Bugbot for commit a7ce236. This will update automatically on new commits. Configure here.