[live-migration] harden resume against stdio and exit wait loss - #2866
Conversation
When a live migration rolls back to the source, the VM resumes but the container's stdout/stderr were dropped during blackout and never restored, so anything watching the container's output saw it stop for good. Resume now brings those streams back the same way the destination already does, so a rolled-back container keeps streaming its output as if the migration had never been attempted. Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
24ab7b6 to
fcd6d96
Compare
A process reopened on the destination relies on the exit wait its previous owner left outstanding in the guest. If the process exited while no bridge was connected, the guest's response to that wait was dropped and never re-sent, so the reopened process would wait for an exit that is never reported. On reopen, probe with a short bounded wait: an already-exited process returns its retained exit code immediately, while a still-running one lets the probe time out and the outstanding wait is watched as before. Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
Shreyansh Sancheti (shreyanshjain7174)
left a comment
There was a problem hiding this comment.
LGTM!
| } | ||
| resp := &prot.ContainerWaitForProcessResponse{} | ||
| if probe, err := p.gc.brdg.AsyncRPC(ctx, prot.RPCWaitForProcess, &req, resp); err == nil { | ||
| probe.Wait() |
There was a problem hiding this comment.
IIUC for a successful migration of an N-container pod, destination Finalize adds ~N × 500 ms, fully serialized, while holding the migration/pod/container locks. Your 10-container test ≈ ~5 s of added resume latency. Because migration.Controller.mu is held throughout, concurrent State()/Cancel()/subscribe calls also block during that window (notably, Cancel can't proceed). Please correct me if that's not the case
The synchronous probe serializes N×500 ms under the finalize locks on every destination resume;
Should we consider running startExitWatch asynchronously (guarding the waitCall/waitResp handoff), or lowering exitProbeTimeoutMs, since the running-process case (the overwhelming majority) always pays the full timeout for no functional benefit
There was a problem hiding this comment.
Thanks for the insights! Yes, this would have hampered the perf as containers scaled.
As discussed offline, reducing the timeout to 50ms seems like a good idea since it balances code complexity with the perf. Addressed the same in latest commit.
Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
Manish Ranjan Mahanta (marma-dev)
left a comment
There was a problem hiding this comment.
LGTM
036d00e
into
microsoft:main
Summary
Two blackout-related gaps in live migration, each restoring behavior the connection drop had broken:
Testing
Ran
live-migrationsource rollback with a single container and with 10 containers; validated logs stream back on the source.