[Disk Manager] Fix disk manager tests after updating StatVolume behaviour #2924
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rattysed
added
large-tests
Rattysed
commented
Jan 24, 2025
cloud/blockstore/libs/storage/service/service_actor_destroy.cpp
Outdated
Show resolved
Hide resolved
Rattysed
commented
Jan 24, 2025
Rattysed
commented
Jan 24, 2025
BarkovBG
reviewed
Jan 24, 2025
cloud/blockstore/libs/storage/volume/volume_actor_statvolume.cpp
Outdated
Show resolved
Hide resolved
BarkovBG
reviewed
Jan 24, 2025
cloud/blockstore/libs/storage/volume/volume_actor_statvolume.cpp
Outdated
Show resolved
Hide resolved
Rattysed
force-pushed
the
user/rattysed/issue-2008
branch
from
76e53ee to
a2e5de5
Compare
Rattysed
changed the title
Rattysed
removed
the
blockstore
Rattysed
force-pushed
the
user/rattysed/issue-2008
branch
from
d5c00e9 to
875bc6d
Compare
Rattysed
force-pushed
the
user/rattysed/issue-2008
branch
from
875bc6d to
15301c7
Compare
BarkovBG
reviewed
Feb 5, 2025
cloud/disk_manager/internal/pkg/dataplane/snapshot/storage/storage_ydb_test.go
Outdated
Show resolved
Hide resolved
cloud/disk_manager/internal/pkg/facade/snapshot_service_test/snapshot_service_test.go
Show resolved
Hide resolved
cloud/disk_manager/internal/pkg/facade/snapshot_service_test/snapshot_service_test.go
Outdated
Show resolved
Hide resolved
BarkovBG
reviewed
Feb 5, 2025
Comment on lines
765
to
766
| // Should wait here because checkpoint is deleted on |createOp| operation | ||
| // cancel (and exact time of this event is unknown). |
Comment on lines
765
to
766
| // Should wait here because checkpoint is deleted on |createOp| operation | ||
| // cancel (and exact time of this event is unknown). |
There was a problem hiding this comment.
- свеху ифа комментарий почему мы так делаем
cloud/disk_manager/internal/pkg/facade/snapshot_service_test/snapshot_service_test.go
Show resolved
Hide resolved
BarkovBG
reviewed
Feb 6, 2025
| @@ -758,13 +764,13 @@ func TestSnapshotServiceDeleteSnapshotWhenCreationIsInFlight(t *testing.T) { | |||
| err = internal_client.WaitOperation(ctx, client, operation.Id) | |||
| require.NoError(t, err) | |||
|
|
|||
| _ = internal_client.WaitOperation(ctx, client, createOp.Id) | |||
| createErr := internal_client.WaitOperation(ctx, client, createOp.Id) | |||
| // TODO: enable this check after resolving issue | ||
| // https://github.com/ydb-platform/nbs/issues/2008. | ||
| // testcommon.WaitForCheckpointsAreEmpty(t, ctx, diskID) | ||
| if createErr != nil { |
There was a problem hiding this comment.
выше ифа надо написать почему мы ждем чекпоинты только если создание успешное
| @@ -604,17 +605,22 @@ func TestSnapshotServiceDeleteIncrementalSnapshotWhileCreating(t *testing.T) { | |||
| err = internal_client.WaitOperation(ctx, client, deleteOperation.Id) | |||
| require.NoError(t, err) | |||
|
|
|||
| //nolint:sa9003 | |||
| // TODO: remove line above after | |||
| // https://github.com/ydb-platform/nbs/issues/2008 | |||
| if creationErr == nil { | |||
There was a problem hiding this comment.
тут сверху ифа нужно рассказать, почему у нас две ветки в зависимости от ошибки создания
There was a problem hiding this comment.
1 - снапшот успел создастся, а затем удалиться - тогда чепоинта не должно быть
Comment on lines
+617
to
+618
| // If there is a record about this disk left in incrementality table, | ||
| // checkpoint that corresponds to base snapshot should not be deleted. |
There was a problem hiding this comment.
тут нужо рассказать, что в случае неуспешного создания может быть два сценария
если мы успели потереть данные про baseSnapshotID из таблицы инкрементальности или нет
типо:
// in case of snapshot creation failure baseSnapshot may be already deleted from incremental table and then checkpoint should not exist on the disk.
// Otherwise checkpoint should exist
что-то такое
из твоего комментария вообще не понятно что происходит и почему так
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#2008 — Fixed Dm tests, that depend on StatVolume.