Skip to content

Commit dc03833

Browse files
iamnalinoriamnalinor
authored
[DM] Add estimates for erasing local disks in disks.DeleteDisk (#4390)
Part of #3738 Add estimated times for `disks.DeleteDisk`. In most cases, the task completes instantly. But on deleting local disks (local-hdd or local-ssd) blockstore needs to erase it, so it takes some time that I am estimating in this PR. Proposing setting the bandwidth for SDD local disks to 500 MiB/s and HDD local disks to 100 MiB/s.
1 parent 39240ce commit dc03833

File tree

5 files changed

+245
-14
lines changed

5 files changed

+245
-14
lines changed

‎cloud/disk_manager/internal/pkg/performance/config/config.proto‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,6 @@ message PerformanceConfig {
1515
optional uint64 TransferFromURLToSnapshotBandwidthMiBs = 4 [default = 200];
1616

1717
optional uint64 CreateDRBasedDiskCheckpointBandwidthMiBs = 5 [default = 300];
18+
optional uint64 SSDLocalDiskDeletingBandwidthMiBs = 6 [default = 500];
19+
optional uint64 HDDLocalDiskDeletingBandwidthMiBs = 7 [default = 100];
1820
}

‎cloud/disk_manager/internal/pkg/services/disks/delete_disk_task.go‎

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
disk_manager "github.com/ydb-platform/nbs/cloud/disk_manager/api"
1010
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/clients/nbs"
1111
dataplane_protos "github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/dataplane/protos"
12+
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/performance"
13+
performance_config "github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/performance/config"
1214
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/resources"
1315
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/services/disks/protos"
1416
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/services/pools"
@@ -22,12 +24,13 @@ import (
2224
////////////////////////////////////////////////////////////////////////////////
2325

2426
type deleteDiskTask struct {
25-
storage resources.Storage
26-
scheduler tasks.Scheduler
27-
poolService pools.Service
28-
nbsFactory nbs.Factory
29-
request *protos.DeleteDiskRequest
30-
state *protos.DeleteDiskTaskState
27+
performanceConfig *performance_config.PerformanceConfig
28+
storage resources.Storage
29+
scheduler tasks.Scheduler
30+
poolService pools.Service
31+
nbsFactory nbs.Factory
32+
request *protos.DeleteDiskRequest
33+
state *protos.DeleteDiskTaskState
3134
}
3235

3336
func (t *deleteDiskTask) Save() ([]byte, error) {
@@ -50,6 +53,11 @@ func (t *deleteDiskTask) deleteDisk(
5053
execCtx tasks.ExecutionContext,
5154
) error {
5255

56+
err := t.setEstimate(ctx, execCtx)
57+
if err != nil {
58+
return err
59+
}
60+
5361
selfTaskID := execCtx.GetTaskID()
5462
diskID := t.request.Disk.DiskId
5563
sync := t.request.Sync
@@ -170,3 +178,61 @@ func (t *deleteDiskTask) GetMetadata(
170178
func (t *deleteDiskTask) GetResponse() proto.Message {
171179
return &empty.Empty{}
172180
}
181+
182+
////////////////////////////////////////////////////////////////////////////////
183+
184+
func (t *deleteDiskTask) setEstimate(
185+
ctx context.Context,
186+
execCtx tasks.ExecutionContext,
187+
) error {
188+
189+
// Only sync requests are blocking.
190+
if !t.request.Sync {
191+
return nil
192+
}
193+
194+
diskID := t.request.Disk.DiskId
195+
196+
diskMeta, err := t.storage.GetDiskMeta(ctx, diskID)
197+
if err != nil {
198+
return err
199+
}
200+
201+
if diskMeta == nil {
202+
// Should be idempotent.
203+
return nil
204+
}
205+
206+
client, err := t.nbsFactory.GetClient(ctx, diskMeta.ZoneID)
207+
if err != nil {
208+
return err
209+
}
210+
211+
diskParams, err := client.Describe(ctx, diskID)
212+
if err != nil {
213+
if nbs.IsDiskNotFoundError(err) {
214+
// Should be idempotent.
215+
return nil
216+
}
217+
218+
return err
219+
}
220+
221+
diskSize := diskParams.BlocksCount * uint64(diskParams.BlockSize)
222+
223+
switch diskParams.Kind {
224+
case types.DiskKind_DISK_KIND_SSD_LOCAL:
225+
execCtx.SetEstimatedInflightDuration(performance.Estimate(
226+
diskSize,
227+
t.performanceConfig.GetSSDLocalDiskDeletingBandwidthMiBs(),
228+
))
229+
230+
case types.DiskKind_DISK_KIND_HDD_LOCAL:
231+
execCtx.SetEstimatedInflightDuration(performance.Estimate(
232+
diskSize,
233+
t.performanceConfig.GetHDDLocalDiskDeletingBandwidthMiBs(),
234+
))
235+
}
236+
237+
return nil
238+
}

‎cloud/disk_manager/internal/pkg/services/disks/delete_disk_task_test.go‎

Lines changed: 163 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ package disks
33
import (
44
"context"
55
"testing"
6+
"time"
67

78
"github.com/stretchr/testify/mock"
89
"github.com/stretchr/testify/require"
10+
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/clients/nbs"
911
nbs_mocks "github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/clients/nbs/mocks"
12+
performance_config "github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/performance/config"
1013
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/resources"
1114
storage_mocks "github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/resources/mocks"
1215
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/services/disks/protos"
@@ -40,20 +43,30 @@ func testDeleteDiskTaskRun(t *testing.T, sync bool) {
4043
state: &protos.DeleteDiskTaskState{},
4144
}
4245

46+
diskMeta := &resources.DiskMeta{
47+
ZoneID: "zone",
48+
Kind: "ssd",
49+
DeleteTaskID: "toplevel_task_id",
50+
}
51+
if sync {
52+
storage.On(
53+
"GetDiskMeta",
54+
ctx,
55+
"disk",
56+
).Return(diskMeta, nil).Once()
57+
}
4358
storage.On(
4459
"DeleteDisk",
4560
ctx,
4661
"disk",
4762
"toplevel_task_id",
4863
mock.Anything,
49-
).Return(&resources.DiskMeta{
50-
ZoneID: "zone",
51-
DeleteTaskID: "toplevel_task_id",
52-
}, nil)
64+
).Return(diskMeta, nil).Once()
5365
storage.On("DiskDeleted", ctx, "disk", mock.Anything).Return(nil)
5466

5567
nbsFactory.On("GetClient", ctx, "zone").Return(nbsClient, nil)
5668
if sync {
69+
nbsClient.On("Describe", ctx, "disk").Return(nbs.DiskParams{}, nil)
5770
nbsClient.On("DeleteSync", ctx, "disk").Return(nil)
5871
} else {
5972
nbsClient.On("Delete", ctx, "disk").Return(nil)
@@ -112,6 +125,7 @@ func TestDeleteDiskTaskRunWithDiskCreatedFromImage(t *testing.T) {
112125
).Return(&resources.DiskMeta{
113126
ZoneID: "zone",
114127
SrcImageID: "image",
128+
Kind: "ssd",
115129
DeleteTaskID: "toplevel_task_id",
116130
}, nil)
117131
storage.On("DiskDeleted", ctx, "disk", mock.Anything).Return(nil)
@@ -171,6 +185,7 @@ func TestDeleteDiskTaskCancel(t *testing.T) {
171185
mock.Anything,
172186
).Return(&resources.DiskMeta{
173187
ZoneID: "zone",
188+
Kind: "ssd",
174189
SrcImageID: "image",
175190
DeleteTaskID: "toplevel_task_id",
176191
}, nil)
@@ -234,3 +249,147 @@ func TestDeleteDiskTaskWithNonExistentDisk(t *testing.T) {
234249
mock.AssertExpectationsForObjects(t, storage, scheduler, poolService, nbsFactory, execCtx)
235250
require.NoError(t, err)
236251
}
252+
253+
func testDeleteDiskTaskEstimatedInflightDurationForLocalDisks(
254+
t *testing.T,
255+
sync bool,
256+
diskKind types.DiskKind,
257+
expectedEstimatedInflightDuration time.Duration,
258+
) {
259+
ctx := context.Background()
260+
261+
SSDLocalDiskDeletingBandwidthMiBs := uint64(100)
262+
HDDLocalDiskDeletingBandwidthMiBs := uint64(10)
263+
performanceConfig := &performance_config.PerformanceConfig{
264+
SSDLocalDiskDeletingBandwidthMiBs: &SSDLocalDiskDeletingBandwidthMiBs,
265+
HDDLocalDiskDeletingBandwidthMiBs: &HDDLocalDiskDeletingBandwidthMiBs,
266+
}
267+
storage := storage_mocks.NewStorageMock()
268+
scheduler := tasks_mocks.NewSchedulerMock()
269+
poolService := pools_mocks.NewServiceMock()
270+
nbsFactory := nbs_mocks.NewFactoryMock()
271+
nbsClient := nbs_mocks.NewClientMock()
272+
execCtx := newExecutionContextMock()
273+
274+
disk := &types.Disk{DiskId: "disk"}
275+
request := &protos.DeleteDiskRequest{Disk: disk, Sync: sync}
276+
277+
task := &deleteDiskTask{
278+
performanceConfig: performanceConfig,
279+
storage: storage,
280+
scheduler: scheduler,
281+
poolService: poolService,
282+
nbsFactory: nbsFactory,
283+
request: request,
284+
state: &protos.DeleteDiskTaskState{},
285+
}
286+
287+
diskMeta := &resources.DiskMeta{
288+
ZoneID: "zone",
289+
DeleteTaskID: "toplevel_task_id",
290+
}
291+
292+
if sync {
293+
storage.On(
294+
"GetDiskMeta",
295+
ctx,
296+
"disk",
297+
).Return(diskMeta, nil).Once()
298+
nbsClient.On("Describe", ctx, "disk").Return(nbs.DiskParams{
299+
Kind: diskKind,
300+
BlocksCount: 100 * 256, // 100 MiB
301+
BlockSize: 4096,
302+
}, nil)
303+
}
304+
305+
storage.On(
306+
"DeleteDisk",
307+
ctx,
308+
"disk",
309+
"toplevel_task_id",
310+
mock.Anything,
311+
).Return(diskMeta, nil).Once()
312+
storage.On("DiskDeleted", ctx, "disk", mock.Anything).Return(nil)
313+
314+
if expectedEstimatedInflightDuration > 0 {
315+
execCtx.On("SetEstimatedInflightDuration", expectedEstimatedInflightDuration)
316+
}
317+
318+
nbsFactory.On("GetClient", ctx, "zone").Return(nbsClient, nil)
319+
if sync {
320+
nbsClient.On("DeleteSync", ctx, "disk").Return(nil)
321+
} else {
322+
nbsClient.On("Delete", ctx, "disk").Return(nil)
323+
}
324+
325+
scheduler.On(
326+
"ScheduleTask",
327+
headers.SetIncomingIdempotencyKey(ctx, "toplevel_task_id_delete_disk_from_incremental"),
328+
"dataplane.DeleteDiskFromIncremental",
329+
"",
330+
mock.Anything,
331+
).Return("deleteTask", nil)
332+
333+
scheduler.On("WaitTask", ctx, execCtx, "deleteTask").Return(nil, nil)
334+
335+
err := task.Run(ctx, execCtx)
336+
mock.AssertExpectationsForObjects(t, storage, scheduler, nbsFactory, nbsClient, execCtx)
337+
require.NoError(t, err)
338+
}
339+
340+
func TestDeleteDiskTaskEstimatedInflightDurationForLocalDisks(t *testing.T) {
341+
testCases := []struct {
342+
name string
343+
sync bool
344+
diskKind types.DiskKind
345+
expectedEstimatedInflightDuration time.Duration
346+
}{
347+
{
348+
name: "No estimate for any disks except local; Sync=false",
349+
sync: false,
350+
diskKind: types.DiskKind_DISK_KIND_SSD,
351+
expectedEstimatedInflightDuration: 0,
352+
},
353+
{
354+
name: "No estimate for any disks except local; Sync=true",
355+
sync: true,
356+
diskKind: types.DiskKind_DISK_KIND_SSD,
357+
expectedEstimatedInflightDuration: 0,
358+
},
359+
{
360+
name: "No estimate for Sync=false; DiskKind=ssd-local",
361+
sync: false,
362+
diskKind: types.DiskKind_DISK_KIND_SSD_LOCAL,
363+
expectedEstimatedInflightDuration: 0,
364+
},
365+
{
366+
name: "Estimate set for Sync=true; DiskKind=ssd-local",
367+
sync: true,
368+
diskKind: types.DiskKind_DISK_KIND_SSD_LOCAL,
369+
expectedEstimatedInflightDuration: 1 * time.Second,
370+
},
371+
{
372+
name: "No estimate for Sync=false; DiskKind=hdd-local",
373+
sync: false,
374+
diskKind: types.DiskKind_DISK_KIND_HDD_LOCAL,
375+
expectedEstimatedInflightDuration: 0,
376+
},
377+
{
378+
name: "Estimate set for Sync=true; DiskKind=hdd-local",
379+
sync: true,
380+
diskKind: types.DiskKind_DISK_KIND_HDD_LOCAL,
381+
expectedEstimatedInflightDuration: 10 * time.Second,
382+
},
383+
}
384+
385+
for _, testCase := range testCases {
386+
t.Run(testCase.name, func(t *testing.T) {
387+
testDeleteDiskTaskEstimatedInflightDurationForLocalDisks(
388+
t,
389+
testCase.sync,
390+
testCase.diskKind,
391+
testCase.expectedEstimatedInflightDuration,
392+
)
393+
})
394+
}
395+
}

‎cloud/disk_manager/internal/pkg/services/disks/register.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/cells"
88
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/clients/nbs"
9+
performance_config "github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/performance/config"
910
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/resources"
1011
disks_config "github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/services/disks/config"
1112
"github.com/ydb-platform/nbs/cloud/disk_manager/internal/pkg/services/pools"
@@ -18,6 +19,7 @@ import (
1819
func RegisterForExecution(
1920
ctx context.Context,
2021
config *disks_config.DisksConfig,
22+
performanceConfig *performance_config.PerformanceConfig,
2123
resourceStorage resources.Storage,
2224
poolStorage storage.Storage,
2325
taskRegistry *tasks.Registry,
@@ -92,10 +94,11 @@ func RegisterForExecution(
9294

9395
err = taskRegistry.RegisterForExecution("disks.DeleteDisk", func() tasks.Task {
9496
return &deleteDiskTask{
95-
storage: resourceStorage,
96-
scheduler: taskScheduler,
97-
poolService: poolService,
98-
nbsFactory: nbsFactory,
97+
performanceConfig: performanceConfig,
98+
storage: resourceStorage,
99+
scheduler: taskScheduler,
100+
poolService: poolService,
101+
nbsFactory: nbsFactory,
99102
}
100103
})
101104
if err != nil {

‎cloud/disk_manager/pkg/app/controlplane.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ func registerControlplaneTasks(
268268
err = disks.RegisterForExecution(
269269
ctx,
270270
config.GetDisksConfig(),
271+
performanceConfig,
271272
resourceStorage,
272273
poolStorage,
273274
taskRegistry,

0 commit comments

Comments
 (0)