retry with new checkpoint if shadow disk failed #3004
Conversation
gy2411
force-pushed
the
users/gayurgin/retry_with_new_checkpoint_id_shadow_disk_failed
branch
from
120c4ed to
0d4a593
Compare
| @@ -94,6 +94,12 @@ func (t *createImageFromDiskTask) run( | |||
| } | |||
|
|
|||
| err = nbsClient.EnsureCheckpointReady(ctx, disk.DiskId, checkpointID) | |||
| if errors.Is(err, errors.NewEmptyRetriableError()) { | |||
There was a problem hiding this comment.
Такая провека ошибки выгдялит не очень здорово. Возможно, стоит из метода EnsureCheckpointReady возвращать статус чекпоинта, а не ошибку? Но в таком случае не понятно, зачем нужен метод EnsureCheckpointReady.
Хотелось, чтобы этот метод помогал убрать дублирование кода между созданием снапшота и созданием образа -- но получается, что он не особо помогает это сделать...
|
Отдельный вопрос -- потом аналогичную правку надо будет сделать и для создания образов. И в текущем виде это приведёт к большому дублированию. Хотелось бы этого избежать. Может, попытаться вынести эту логику в dataplane? Но не текущем этапе предлагаю добить текушую правку, а уже потом разобраться, как это улучшить. |
| //////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| func (t *createSnapshotFromDiskTask) makeCheckpointID(index int) string { | ||
| return fmt.Sprintf("%v_%v", t.request.DstSnapshotId, index) |
There was a problem hiding this comment.
Важно: теперь id чекпоинта может не совпадать с id соответствующего снапшота! Насколько я могу судить по коду, мы нигде не закладывались на то, что они совпадают.
| if err != nil { | ||
| return err | ||
| } | ||
| if t.state.CheckpointID == "" { |
There was a problem hiding this comment.
Мотивировка этого if (и вообще мотивировка поля CheckpointID) -- мы не должны обновлять чекпоинт, если мы уже зашедуллили dataplane таск с предыдущим чекпоинтом.
| ) | ||
| } | ||
|
|
||
| func TestSnapshotServiceCreateSnapshotFromDiskWithFailedShadowDiskLong(t *testing.T) { |
There was a problem hiding this comment.
Сделал два варианта теста с разными границами на интервал случайной зажержки, чтобы меньше зависеть от конкретных таймингов работы таска.
#1950
Сейчас есть баг при создании снапшотов из disk registy based дисков. Чекпоинты для таких дисков делаются через теневой диск (shadow disk). Сейчас, если вот тут мы от чекпоинта с теневым диском получаем статус Error, то мы удаляем чекпоинт и ретраим таск. При ретрае таск падает при попытке создать чекпоинт с тем же id, что был у удалённого чекпоинта.
Теперь вместо этого мы при ретрае создаём чекпоинт с новым checkpoint id.
Важно! Эта правка ещё не полностью решает проблему с падением теневого диска. Она не покрывает случай, когда теневой диск успешно налился, но упал походу наливки снапшота в рамках dataplane таска.