[Disk Manager] retry with new checkpoint id when create snapshot if shadow disk failed during filling #2691
Conversation
…f shadow disk failed during its filling
| if err != nil { | ||
| return err | ||
| } | ||
| if t.state.FinalCheckpointID == "" { |
There was a problem hiding this comment.
Мотивировка этого if (и вообще мотивировка поля FinalCheckpointID) -- мы не должны обновлять чекпоинт, если мы уже зашедуллили dataplane таск с предыдущим чекпоинтом.
|
|
||
| // NBS-1873: should always delete checkpoint. | ||
| err = nbsClient.DeleteCheckpoint(ctx, disk.DiskId, checkpointID) | ||
| err = t.deletePreviousCheckpoint(ctx, nbsClient) |
There was a problem hiding this comment.
Предыдущий чекпоинт тоже надо удалить -- таск мог пойти на отмену после инкремента FailedCheckpointsCount, но до удаления старого чекпоинта.
| ) | ||
| } | ||
|
|
||
| func TestCreateSnapshotFromDiskWithFailedShadowDiskLong(t *testing.T) { |
There was a problem hiding this comment.
Сделал два варианта теста с разными границами на интервал случайной зажержки, чтобы меньше зависеть от конкретных таймингов работы таска.
| } | ||
|
|
||
| input := fmt.Sprintf( | ||
| "{\"DisableAgent\":{\"AgentId\":\"%v\",\"DeviceUUIDs\":%v},\"Message\":\"%v\"}", |
There was a problem hiding this comment.
Хотя ручка и называется "DisableAgent", она не будет ломать весь агент, если ей передать непустой спасок девайсов. Она сломает только девайсы из этого списка.
Сломает -- значит, девайсы начнут отдавать ошибку в ответ на все запросы чтения и записи.
| @@ -278,6 +293,7 @@ func (t *createSnapshotFromDiskTask) GetResponse() proto.Message { | |||
|
|
|||
| func (t *createSnapshotFromDiskTask) ensureCheckpointReady( | |||
There was a problem hiding this comment.
Тут есть конфликт с #2612 (там эта функция уносится в метод nbs-клиента). Придётся его порезолвить.
Кстати, в том pr делается удаление чекпоинта в методе EnsureCheckpointReady, а в этом pr предлагается вынести его за пределы EnsureCheckpointReady. Это важно: иначе будет возможен плохой сценарий, при котором таск пойдёт на ретрай уже после удаления чекпоинта, но еще не успев увеличить FailedCheckpointsCount.
| @@ -75,6 +75,7 @@ def __init__( | |||
| storage_config_patch.DisableLocalService = False | |||
| storage_config_patch.InactiveClientsTimeout = 60000 # 1 min | |||
| storage_config_patch.AgentRequestTimeout = 5000 # 5 sec | |||
| storage_config_patch.UseShadowDisksForNonreplDiskCheckpoints = True | |||
There was a problem hiding this comment.
Возможно, включение этой опции стоит сделать в отдельном pr (вот он: #2687)
…s update when retry shadow disk fail
#1950
Сейчас есть баг при создании снапшотов из disk registy based дисков. Чекпоинты для таких дисков делаются через теневой диск (shadow disk). Сейчас, если вот тут мы от чекпоинта с теневым диском получаем статус Error, то мы удаляем чекпоинт и ретраим таск. При ретрае таск падает при попытке создать чекпоинт с тем же id, что был у удалённого чекпоинта.
Теперь вместо этого мы при ретрае создаём чекпоинт с новым checkpoint id.
Также для воспроизведения падений теневого диска в интеграционных тестах нам нужно ходить в disk registry. Добавляю в nbs client нужный для этого код.
См. больше деталей в комментариях в #1950
Надо понимать, что эта правка ещё не полностью решает проблему с падением теневого диска. Подробнее написал в issue #1950.