fix: skip empty node name in local snapshot info after pause finalization#329
fix: skip empty node name in local snapshot info after pause finalization#329Mesut Oezdil (mesutoezdil) wants to merge 3 commits into
Conversation
|
Mesut Oezdil (@mesutoezdil) - could you please explain how you was able to reproduce the issue, since the proposed fix will not work. The root cause of the issue is somewhere else. |
here are the reproduction steps, i wrote 2 unit tests: T1 // (TestFinalizePausedStep_WorkerGone): worker pod is gone from db before FinalizePausedStep runs. GetWorker returns ErrNotFound. 
see your point about root cause. Even with this fix, resume could pick a wrong node and snapshot restore fails. |
|
Zoe Zhao (@zoez7) is working on a PR introducing the CRASHED state. Could you please wait for her PR to be submitted and after it make a change to move the actor to this stage in case of this condition. |
Mesut Oezdil (mesutoezdil)
force-pushed
the
fix/pause-empty-nodename
branch
2 times, most recently
from
f14d87d to
907531c
Compare
|
Dmitry Berkovich (@dberkov) since #353 landed with the CRASHED state, I've updated this PR: when the node name is still unknown after finalize pause (worker gone from DB, or worker present but with no node name recorded), the actor is now moved to CRASHED instead of PAUSED, per your TODO. Also made CRASHED a terminal state for the FinalizePaused step so the workflow doesn't retry forever. Added a test covering the crash path. |
Without a node name the local snapshot can never be resumed since findFreeWorker would search for a worker on an unknown node forever. Move the actor to CRASHED instead of leaving it stuck in PAUSED. CRASHED is now a terminal state for the workflow, mirroring PAUSED.
Mesut Oezdil (mesutoezdil)
force-pushed
the
fix/pause-empty-nodename
branch
from
d2f4152 to
60ac20b
Compare
2 participants
When the worker pod is gone from the DB during pause finalization, nodeName stays empty.
The code still wrote NodeVmsWithLocalSnapshots: []string{""}, so on the next ResumeActor the scheduler looked for a worker with node name "", found none, and returned "no free workers available" permanently.
Fix: only set NodeVmsWithLocalSnapshots when nodeName is non-empty.