-
Notifications
You must be signed in to change notification settings - Fork 2
Improve snapshotting functionality #29
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
base: v0.25_reference
Are you sure you want to change the base?
Improve snapshotting functionality #29
Conversation
…ns and restorations Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
…pace Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
…m network namespace Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
…pshots Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
proto/types.proto
Outdated
| message FirecrackerMachineConfiguration { | ||
| string CPUTemplate = 1; // Specifies the cpu template. Example: "T2" or "C3" | ||
| bool HtEnabled = 2; // Specifies if hyper-threading should be enabled | ||
| bool TrackDirtyPages = 3; // Specified if dirty-page-tracking should be enabled. |
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.
please move to the 5th position to allow backwards compatibility.
proto/types.proto
Outdated
| // for a microVM should be large enough | ||
| uint32 MemSizeMib = 3; | ||
| uint32 VcpuCount = 4; // Specifies the number of vCPUs for the VM | ||
| uint32 MemSizeMib = 4; |
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.
revert
runtime/service.go
Outdated
|
|
||
| // Once the VM is ready, also start forwarding events from it to our exchange | ||
| attachCh := eventbridge.Attach(ctx, s.eventBridgeClient, s.eventExchange) | ||
| if ! s.snapLoaded { |
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.
refactor for a better code style:
if s.snapLoaded { return }
<...> // everything just like before but without an indent
runtime/service.go
Outdated
|
|
||
| // shutdownSnapLoadedVm shuts down a vm that has been loaded from a snapshot | ||
| func (s *service) shutdownSnapLoadedVm() error { | ||
| // Kill firecracker process and its shild processes |
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.
| // Kill firecracker process and its shild processes | |
| // Kill firecracker process and its child processes |
runtime/service.go
Outdated
| DialTimeout: 100 * time.Millisecond, | ||
| RequestTimeout: 10 * time.Second, | ||
| ResponseHeaderTimeout: 10 * time.Second, | ||
| DialTimeout: 1000 * time.Millisecond, |
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.
is there a reason for this timeout increasing? why to those values?
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.
I noticed some timeouts when restoring or loading snapshots, as far as I remember especially for larger memory sizes which makes sense. The values are chosen quite arbitrarily.
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.
try decreasing them (e.g., try 2x vs 10x as of now). large timeouts create issues in the control plane, the system becomes less responsive
runtime/service.go
Outdated
|
|
||
| // Offload Shuts down a VM and deletes the corresponding firecracker socket | ||
| // and vsock. All of the other resources will persist | ||
| // and vsock. All of the other resources will persist. Depracated! |
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.
| // and vsock. All of the other resources will persist. Depracated! | |
| // and vsock. All of the other resources will persist. DEPRECATED! |
Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
d6e4a75 to
aff3d2b
Compare
0e94206 to
032bd10
Compare
Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
032bd10 to
45eba1d
Compare
ustiugov
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.
lgtm
Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
…cker-containerd into v0.25_reference Signed-off-by: Amory Hoste <amory.hoste@gmail.com>
ec1680b to
19c96c0
Compare
This PR adds the following changes