[live-migration] Fix source resume hang when cancelled before blackout - #2871
[live-migration] Fix source resume hang when cancelled before blackout#2871Harsh Rawat (rawahars) wants to merge 1 commit into
Conversation
On a source rollback, Resume unconditionally re-armed the log listener and re-accepted the guest GCS bridge, assuming a blackout had dropped them. When the migration is cancelled before blackout (e.g. cancel after PrepareAndExport), those connections are still live, so both accepts block until the GCS connection timeout. finalize(resume) then fails with DeadlineExceeded, leaving the session Cancelled and blocking cleanup. We now track whether the bridge transport is still live and, on resume, reuse the existing bridge and log stream instead of re-accepting when it never dropped. Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
47da4cc to
90a1384
Compare
Manish Ranjan Mahanta (marma-dev)
left a comment
Manish Ranjan Mahanta (marma-dev)
left a comment
There was a problem hiding this comment.
LGTM
| return fmt.Errorf("resume source guest connection: %w", err) | ||
| // A source rollback before blackout never dropped the guest connection, so the | ||
| // live bridge and its log stream are reused instead of re-accepted. | ||
| reuseLiveConn := rebuildBridge && c.guest.IsBridgeConnected() |
There was a problem hiding this comment.
Is there a potential race: connected is read here as a snapshot, but it is not synchronized with the recvLoopRoutine transition. If recvLoop() returns immediately before connected.Store(false) executes, Resume() can observe connected == true, set reuseLiveConn, and skip rebuilding the bridge. It can then clear migrating, after which recvLoopRoutine stores connected = false, observes migrating == false, and calls kill(err). This can leave the resumed session using a bridge that is subsequently terminated.
There was a problem hiding this comment.
In short, this workflow would be similar to how a pod/container is handled if the GCS connection was dropped randomly at any point today. Hence it should be safe, without introducing any backward incompatibility.
Long version:
So you are right that IsBridgeConnected is a point in time snapshot of the bridge state. Right now we are using it for Resume API which is a high level API from Controller. We can run into this API from 2 points-
Destination Resume:rebuildBridgeisfalseand hence we would go into the loop. There is no prior bridge connection and therefore, we do that inside if.Source Rollback:- After
blackout: VM was paused and hence listeners dropped and bridge would have collapsed duringTransferAPI causingconnectedto be false. Therefore, it would always return false here and lead us to go inside the if. - Before
blackout:-
Bridge is active at the time of Resume: connected is true and we skip the if.
-
As soon as the
c.guest.IsBridgeConnected()is read inside Resume, the VM terminates or connection drops for some reason.
This is the race scenario but it's safe and follows the existing workflow. If this happens then we would mark the bridge as not migrating and finish thefinalize. Since the GCS connection is not present, the bridge collapses onmigrating == falseand all the processes/containers signal their EXIT.containerdgets the notification and trigger delete. The caller sees the container asEXITEDand pod asNOTREADY. Caller can then shut the shim.This would be similar to the scenario where no migration is going on and the GCS connection dropped mid-way which causes the bridge to collapse and all processes to trigger their
EXIT.
-
- After
3 participants
On a source rollback, Resume unconditionally re-armed the log listener and re-accepted the guest GCS bridge, assuming a blackout had dropped them. When the migration is cancelled before blackout (e.g. cancel after PrepareAndExport), those connections are still live, so both accepts block until the GCS connection timeout. finalize(resume) then fails with DeadlineExceeded, leaving the session Cancelled and blocking cleanup.
We now track whether the bridge transport is still live and, on resume, reuse the existing bridge and log stream instead of re-accepting when it never dropped.