checkoint coordinator: handle failure on saving zero checkpoint #13917
Conversation
yumkam
changed the title
| @@ -372,11 +372,12 @@ void TCheckpointCoordinator::Handle(const TEvCheckpointCoordinator::TEvScheduleC | |||
| CC_LOG_D("Got TEvScheduleCheckpointing"); | |||
| ScheduleNextCheckpoint(); | |||
| const auto checkpointsInFly = PendingCheckpoints.size() + PendingCommitCheckpoints.size(); | |||
| if (checkpointsInFly >= Settings.GetMaxInflight() || InitingZeroCheckpoint) { | |||
| if (checkpointsInFly >= Settings.GetMaxInflight() || (InitingZeroCheckpoint && !FailedZeroCheckpoint)) { | |||
There was a problem hiding this comment.
Тогда тесты неправильно написаны, есть тест ShouldAbortPreviousCheckpointsIfNodeStateCantBeSaved.
В нем есть MockRunGraph(), которого видимо не должно быть. А когда он должен приходить непонятно
There was a problem hiding this comment.
Он должен приходить после сохранения ZeroCheckpoint (а в тесте есть только симуляция одного -- первого -- сохранения чекпоинта -- неуспешная); никакого влияния ни на что это не оказывает, но правильнее его убрать
There was a problem hiding this comment.
В принципе, ещё можно добавить ассерты в Handle(EvRunGraph).
Y_ABORT_UNLESS(InitingZeroCheckpoint);
Y_ABORT_UNLESS(!FailedZeroCheckpoint);
В норме оно вроде должно работать (TEvZeroCheckpointDone->SetLoadFromCheckpointMode->Ping(SetLoadFrom...Cookie)->EvRunGraph), но в тестах что-то налажано и они с этим падают :-|
There was a problem hiding this comment.
Разобрался с тестами, добавил assert для не-релиза
There was a problem hiding this comment.
Так теперь если ZeroCheckpoint фейлится, то TEvZeroCheckpointDone не отправляется и граф не запускается. А чекпойнты начнут идти. Может лучше граф явно зафейлить.
There was a problem hiding this comment.
Если я правильно понял TODO, то граф работает сразу. Но раньше несохранившийся zero checkpoint ломал чекпоинты вообще, а теперь первый же сохранившийся просто играет роль zero checkpoint.
Если ошибка транзиентная, и просто повторение поможет -- всё отлично.
Если ошибка не транзиентная, и чекпоинты вообще не сохраняются, никакой разницы с тем, что это начало происходить после zero checkpoint: нужны алерты за failure rate.
There was a problem hiding this comment.
Как всё запутано) Если граф сразу запускается, тогда InitingZeroCheckpoint означает просто признак необходимости отравить TEvZeroCheckpointDone при первом же успешном чекпойнте. И в условии checkpointsInFly >= Settings.GetMaxInflight() все эти флаги не нужны.
There was a problem hiding this comment.
Там ещё отдельная ветка с восстановлением из checkpoint, там тоже выставляется InitingZeroCheckpoint
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
... on simulating save failure. Only one (zero'th) checkpoint save is simulated; as it simulates failure, EvZeroCheckpointDone is not expected to be sent: Expected calls sequence is EvZeroCheckpointDone->EvForwardPing(SetLoadMode...Cookie)->EvRunGraph
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
yumkam
changed the title
yumkam
changed the title
…platform#13917) (cherry picked from commit 755e82b)
…nt (ydb-platform#13917)" This reverts commit 755e82b.
Changelog entry
...
Changelog category
Additional information
...