From b8e25873600c52442d55919f577193b0ac36dbed Mon Sep 17 00:00:00 2001 From: yegorskii Date: Thu, 21 Nov 2024 16:56:37 +0300 Subject: [PATCH] issue-2489: limit async get duration (#2490) * issue-2489: limit async get duration * update * add ydb patch * update * update * better part2 ut --- cloud/blockstore/config/storage.proto | 3 + cloud/blockstore/libs/storage/core/config.cpp | 3 +- cloud/blockstore/libs/storage/core/config.h | 3 + .../libs/storage/core/disk_counters.h | 5 ++ .../partition/part_actor_compaction.cpp | 16 ++++- .../storage/partition/part_actor_readblob.cpp | 4 ++ .../libs/storage/partition/part_ut.cpp | 52 ++++++++++++++++ .../blockstore/libs/storage/partition/ya.make | 1 + .../partition2/part2_actor_compaction.cpp | 16 ++++- .../partition2/part2_actor_readblob.cpp | 14 +++++ .../storage/partition2/part2_events_private.h | 1 + .../libs/storage/partition2/part2_ut.cpp | 60 ++++++++++++++++++- .../libs/storage/partition2/ya.make | 7 ++- .../partition_common/actor_read_blob.cpp | 8 +++ .../partition_common/actor_read_blob.h | 2 + .../storage/partition_common/events_private.h | 1 + 16 files changed, 189 insertions(+), 7 deletions(-) diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto index 1cb05555c5a..1b11a2e364a 100644 --- a/cloud/blockstore/config/storage.proto +++ b/cloud/blockstore/config/storage.proto @@ -1053,4 +1053,7 @@ message TStorageServiceConfig // agents devices and will not suspend local devices. Instead, PURGE_HOST // should be used for these purposes. optional bool DiskRegistryAlwaysAllocatesLocalDisks = 388; + + optional uint32 BlobStorageAsyncGetTimeoutHDD = 389; + optional uint32 BlobStorageAsyncGetTimeoutSSD = 390; } diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp index 3333bef565d..dc0e4294fc3 100644 --- a/cloud/blockstore/libs/storage/core/config.cpp +++ b/cloud/blockstore/libs/storage/core/config.cpp @@ -512,7 +512,8 @@ TDuration MSeconds(ui32 value) xxx(IdleAgentDeployByCmsDelay, TDuration, Hours(1) )\ xxx(AllowLiteDiskReallocations, bool, false )\ xxx(DiskRegistryDisksNotificationTimeout, TDuration, Seconds(5) )\ - + xxx(BlobStorageAsyncGetTimeoutHDD, TDuration, Seconds(0) )\ + xxx(BlobStorageAsyncGetTimeoutSSD, TDuration, Seconds(0) )\ // BLOCKSTORE_STORAGE_CONFIG_RW diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h index fab3bb47742..288304cbe89 100644 --- a/cloud/blockstore/libs/storage/core/config.h +++ b/cloud/blockstore/libs/storage/core/config.h @@ -608,6 +608,9 @@ class TStorageConfig TString GetNodeType() const; NCloud::NProto::TConfigDispatcherSettings GetConfigDispatcherSettings() const; + + TDuration GetBlobStorageAsyncGetTimeoutHDD() const; + TDuration GetBlobStorageAsyncGetTimeoutSSD() const; }; ui64 GetAllocationUnit( diff --git a/cloud/blockstore/libs/storage/core/disk_counters.h b/cloud/blockstore/libs/storage/core/disk_counters.h index 9a1d2e21ec6..4772d2c1bcd 100644 --- a/cloud/blockstore/libs/storage/core/disk_counters.h +++ b/cloud/blockstore/libs/storage/core/disk_counters.h @@ -207,6 +207,10 @@ struct TSimpleDiskCounters EPublishingPolicy::Repl, TSimpleCounter::ECounterType::Generic, ECounterExpirationPolicy::Permanent}; + TCounter ReadBlobDeadlineCount{ + EPublishingPolicy::Repl, + TSimpleCounter::ECounterType::Generic, + ECounterExpirationPolicy::Expiring}; // DiskRegistry based TCounter HasBrokenDevice{ @@ -251,6 +255,7 @@ struct TSimpleDiskCounters MakeMeta<&TSimpleDiskCounters::CompactionRangeCountPerRun>(), MakeMeta<&TSimpleDiskCounters::UnconfirmedBlobCount>(), MakeMeta<&TSimpleDiskCounters::ConfirmedBlobCount>(), + MakeMeta<&TSimpleDiskCounters::ReadBlobDeadlineCount>(), MakeMeta<&TSimpleDiskCounters::HasBrokenDevice>(), MakeMeta<&TSimpleDiskCounters::HasBrokenDeviceSilent>(), diff --git a/cloud/blockstore/libs/storage/partition/part_actor_compaction.cpp b/cloud/blockstore/libs/storage/partition/part_actor_compaction.cpp index 7187ad0251c..4b7c27e591f 100644 --- a/cloud/blockstore/libs/storage/partition/part_actor_compaction.cpp +++ b/cloud/blockstore/libs/storage/partition/part_actor_compaction.cpp @@ -148,6 +148,7 @@ class TCompactionActor final const ui32 MaxBlocksInBlob; const ui32 MaxAffectedBlocksPerCompaction; const IBlockDigestGeneratorPtr BlockDigestGenerator; + const TDuration ReadBlobTimeout; const ui64 CommitId; @@ -179,6 +180,7 @@ class TCompactionActor final ui32 maxBlocksInBlob, ui32 maxAffectedBlocksPerCompaction, IBlockDigestGeneratorPtr blockDigestGenerator, + TDuration readBlobTimeout, ui64 commitId, TVector rangeCompactionInfos, TVector requests); @@ -239,6 +241,7 @@ TCompactionActor::TCompactionActor( ui32 maxBlocksInBlob, ui32 maxAffectedBlocksPerCompaction, IBlockDigestGeneratorPtr blockDigestGenerator, + TDuration readBlobTimeout, ui64 commitId, TVector rangeCompactionInfos, TVector requests) @@ -249,6 +252,7 @@ TCompactionActor::TCompactionActor( , MaxBlocksInBlob(maxBlocksInBlob) , MaxAffectedBlocksPerCompaction(maxAffectedBlocksPerCompaction) , BlockDigestGenerator(std::move(blockDigestGenerator)) + , ReadBlobTimeout(readBlobTimeout) , CommitId(commitId) , RangeCompactionInfos(std::move(rangeCompactionInfos)) , Requests(std::move(requests)) @@ -387,6 +391,10 @@ void TCompactionActor::ReadBlocks(const TActorContext& ctx) current.GroupId); } + const auto readBlobDeadline = ReadBlobTimeout ? + ctx.Now() + ReadBlobTimeout : + TInstant::Max(); + for (ui32 batchIndex = 0; batchIndex < BatchRequests.size(); ++batchIndex) { auto& batch = BatchRequests[batchIndex]; if (batch.UnchangedBlobOffsets) { @@ -423,7 +431,7 @@ void TCompactionActor::ReadBlocks(const TActorContext& ctx) std::move(subSgList), batch.GroupId, true, // async - TInstant::Max(), // deadline + readBlobDeadline, // deadline shouldCalculateChecksums ); @@ -1949,6 +1957,11 @@ void TPartitionActor::CompleteCompaction( } } + auto readBlobTimeout = + PartitionConfig.GetStorageMediaKind() == NProto::STORAGE_MEDIA_SSD ? + Config->GetBlobStorageAsyncGetTimeoutSSD() : + Config->GetBlobStorageAsyncGetTimeoutHDD(); + auto actor = NCloud::Register( ctx, args.RequestInfo, @@ -1958,6 +1971,7 @@ void TPartitionActor::CompleteCompaction( State->GetMaxBlocksInBlob(), Config->GetMaxAffectedBlocksPerCompaction(), BlockDigestGenerator, + readBlobTimeout, args.CommitId, std::move(rangeCompactionInfos), std::move(requests)); diff --git a/cloud/blockstore/libs/storage/partition/part_actor_readblob.cpp b/cloud/blockstore/libs/storage/partition/part_actor_readblob.cpp index fda89850fd6..fd2bdbc7910 100644 --- a/cloud/blockstore/libs/storage/partition/part_actor_readblob.cpp +++ b/cloud/blockstore/libs/storage/partition/part_actor_readblob.cpp @@ -99,6 +99,10 @@ void TPartitionActor::HandleReadBlobCompleted( return; } + if (msg->DeadlineSeen) { + PartCounters->Simple.ReadBlobDeadlineCount.Increment(1); + } + if (State->IncrementReadBlobErrorCount() >= Config->GetMaxReadBlobErrorsBeforeSuicide()) { diff --git a/cloud/blockstore/libs/storage/partition/part_ut.cpp b/cloud/blockstore/libs/storage/partition/part_ut.cpp index 80beff690f0..545a24aa774 100644 --- a/cloud/blockstore/libs/storage/partition/part_ut.cpp +++ b/cloud/blockstore/libs/storage/partition/part_ut.cpp @@ -34,6 +34,7 @@ #include #include +#include #include #include @@ -11162,6 +11163,57 @@ Y_UNIT_TEST_SUITE(TPartitionTest) UNIT_ASSERT_VALUES_EQUAL(0, compactionByBlockCount); } + + Y_UNIT_TEST(ShouldAbortCompactionIfReadBlobFailsWithDeadlineExceeded) + { + NProto::TStorageServiceConfig config; + config.SetBlobStorageAsyncGetTimeoutHDD(TDuration::Seconds(1).MilliSeconds()); + auto runtime = PrepareTestActorRuntime(config); + + TPartitionClient partition(*runtime); + partition.WaitReady(); + + partition.WriteBlocks(3, 33); + partition.Flush(); + + ui32 failedReadBlob = 0; + runtime->SetEventFilter([&] + (TTestActorRuntimeBase& runtime, TAutoPtr& ev) + { + Y_UNUSED(runtime); + + if (ev->GetTypeRewrite() == TEvBlobStorage::EvVGet) { + const auto* msg = ev->Get(); + if (msg->Record.GetHandleClass() == NKikimrBlobStorage::AsyncRead && + msg->Record.GetMsgQoS().HasDeadlineSeconds()) + { + return true; + } + } else if (ev->GetTypeRewrite() == TEvStatsService::EvVolumePartCounters) { + auto* msg = + ev->Get(); + failedReadBlob = + msg->DiskCounters->Simple.ReadBlobDeadlineCount.Value; + } + return false; + }); + + partition.SendCompactionRequest(); + runtime->AdvanceCurrentTime(TDuration::Seconds(1)); + auto response = partition.RecvCompactionResponse(); + UNIT_ASSERT_VALUES_EQUAL(E_REJECTED, response->GetError().GetCode()); + + runtime->AdvanceCurrentTime(TDuration::Seconds(15)); + + partition.SendToPipe( + std::make_unique()); + { + TDispatchOptions options; + options.FinalEvents.emplace_back(TEvStatsService::EvVolumePartCounters); + runtime->DispatchEvents(options); + } + UNIT_ASSERT_VALUES_EQUAL(1, failedReadBlob); + } } } // namespace NCloud::NBlockStore::NStorage::NPartition diff --git a/cloud/blockstore/libs/storage/partition/ya.make b/cloud/blockstore/libs/storage/partition/ya.make index ffdf7b50245..99e69d9cc74 100644 --- a/cloud/blockstore/libs/storage/partition/ya.make +++ b/cloud/blockstore/libs/storage/partition/ya.make @@ -73,6 +73,7 @@ PEERDIR( library/cpp/monlib/service/pages contrib/ydb/core/base + contrib/ydb/core/blobstorage contrib/ydb/core/node_whiteboard contrib/ydb/core/scheme contrib/ydb/core/tablet diff --git a/cloud/blockstore/libs/storage/partition2/part2_actor_compaction.cpp b/cloud/blockstore/libs/storage/partition2/part2_actor_compaction.cpp index 23d2187ce35..d6a9a01fc6c 100644 --- a/cloud/blockstore/libs/storage/partition2/part2_actor_compaction.cpp +++ b/cloud/blockstore/libs/storage/partition2/part2_actor_compaction.cpp @@ -72,6 +72,7 @@ class TCompactionActor final const TActorId Tablet; const ui64 CommitId; const TBlockRange32 BlockRange; + const TDuration ReadBlobTimeout; TGarbageInfo GarbageInfo; TAffectedBlobInfos AffectedBlobInfos; ui32 BlobsSkipped; @@ -107,6 +108,7 @@ class TCompactionActor final const TActorId& tablet, ui64 commitId, const TBlockRange32& blockRange, + TDuration readBlobTimeout, TGarbageInfo garbageInfo, TAffectedBlobInfos affectedBlobInfos, ui32 blobsSkipped, @@ -162,6 +164,7 @@ TCompactionActor::TCompactionActor( const TActorId& tablet, ui64 commitId, const TBlockRange32& blockRange, + TDuration readBlobTimeout, TGarbageInfo garbageInfo, TAffectedBlobInfos affectedBlobInfos, ui32 blobsSkipped, @@ -175,6 +178,7 @@ TCompactionActor::TCompactionActor( , Tablet(tablet) , CommitId(commitId) , BlockRange(blockRange) + , ReadBlobTimeout(readBlobTimeout) , GarbageInfo(std::move(garbageInfo)) , AffectedBlobInfos(std::move(affectedBlobInfos)) , BlobsSkipped(blobsSkipped) @@ -269,6 +273,10 @@ void TCompactionActor::ReadBlocks(const TActorContext& ctx) { bool readBlobSent = false; + const auto readBlobDeadline = ReadBlobTimeout ? + ctx.Now() + ReadBlobTimeout : + TInstant::Max(); + ui32 requestIndex = 0; for (auto& req: ReadRequests) { if (req.DataBlobOffsets) { @@ -285,7 +293,7 @@ void TCompactionActor::ReadBlocks(const TActorContext& ctx) req.BlobContent.GetGuardedSgList(), req.GroupId, true, // async - TInstant::Max() // deadline + readBlobDeadline // deadline ); if (!RequestInfo->CallContext->LWOrbit.Fork(request->CallContext->LWOrbit)) { @@ -1164,6 +1172,11 @@ void TPartitionActor::CompleteCompaction( blobIndex++); } + auto readBlobTimeout = + PartitionConfig.GetStorageMediaKind() == NProto::STORAGE_MEDIA_SSD ? + Config->GetBlobStorageAsyncGetTimeoutSSD() : + Config->GetBlobStorageAsyncGetTimeoutHDD(); + auto actor = NCloud::Register( ctx, args.RequestInfo, @@ -1173,6 +1186,7 @@ void TPartitionActor::CompleteCompaction( SelfId(), args.CommitId, args.BlockRange, + readBlobTimeout, std::move(args.GarbageInfo), std::move(args.AffectedBlobInfos), args.BlobsSkipped, diff --git a/cloud/blockstore/libs/storage/partition2/part2_actor_readblob.cpp b/cloud/blockstore/libs/storage/partition2/part2_actor_readblob.cpp index 6658dba6843..4516d58748e 100644 --- a/cloud/blockstore/libs/storage/partition2/part2_actor_readblob.cpp +++ b/cloud/blockstore/libs/storage/partition2/part2_actor_readblob.cpp @@ -40,6 +40,8 @@ class TReadBlobActor final TInstant RequestSent; TInstant ResponseReceived; + bool DeadlineSeen = false; + public: TReadBlobActor( TRequestInfoPtr requestInfo, @@ -159,6 +161,10 @@ void TReadBlobActor::NotifyCompleted( request->RequestTime = ResponseReceived - RequestSent; request->GroupId = Request->GroupId; + if (DeadlineSeen) { + request->DeadlineSeen = true; + } + NCloud::Send(ctx, Tablet, std::move(request)); } @@ -191,6 +197,10 @@ void TReadBlobActor::ReplyError( description.data(), response.Print(false).data()); + if (response.Status == NKikimrProto::DEADLINE) { + DeadlineSeen = true; + } + auto error = MakeError(E_REJECTED, "TEvBlobStorage::TEvGet failed: " + description); ReplyAndDie(ctx, std::make_unique(error)); } @@ -411,6 +421,10 @@ void TPartitionActor::HandleReadBlobCompleted( return; } + if (msg->DeadlineSeen) { + PartCounters->Simple.ReadBlobDeadlineCount.Increment(1); + } + if (State->IncrementReadBlobErrorCount() >= Config->GetMaxReadBlobErrorsBeforeSuicide()) { diff --git a/cloud/blockstore/libs/storage/partition2/part2_events_private.h b/cloud/blockstore/libs/storage/partition2/part2_events_private.h index 969c14659d9..bf35df53dae 100644 --- a/cloud/blockstore/libs/storage/partition2/part2_events_private.h +++ b/cloud/blockstore/libs/storage/partition2/part2_events_private.h @@ -123,6 +123,7 @@ struct TEvPartitionPrivate ui32 BytesCount = 0; TDuration RequestTime; ui32 GroupId = 0; + bool DeadlineSeen = false; TReadBlobCompleted() = default; diff --git a/cloud/blockstore/libs/storage/partition2/part2_ut.cpp b/cloud/blockstore/libs/storage/partition2/part2_ut.cpp index 8067b272cde..c5b29ccab4e 100644 --- a/cloud/blockstore/libs/storage/partition2/part2_ut.cpp +++ b/cloud/blockstore/libs/storage/partition2/part2_ut.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include +#include #include #include @@ -78,6 +80,7 @@ class TDummyActor final constexpr TDuration WaitTimeout = TDuration::Seconds(5); constexpr ui32 DataChannelOffset = 3; +const TActorId VolumeActorId(0, "VVV"); TString GetBlockContent(char fill = 0, size_t size = DefaultBlockSize) { @@ -191,7 +194,7 @@ void InitTestActorRuntime( partConfig, storageAccessMode, 1, // siblingCount - {} // volumeActorId + VolumeActorId ); return tablet.release(); }; @@ -263,6 +266,10 @@ std::unique_ptr PrepareTestActorRuntime( TActorSetupCmd(volumeProxy.release(), TMailboxType::Simple, 0)); } + runtime->AddLocalService( + VolumeActorId, + TActorSetupCmd(new TDummyActor, TMailboxType::Simple, 0)); + runtime->AddLocalService( MakeHiveProxyServiceId(), TActorSetupCmd(new TDummyActor, TMailboxType::Simple, 0) @@ -7026,6 +7033,57 @@ Y_UNIT_TEST_SUITE(TPartition2Test) "tablet is shutting down", response->GetErrorReason()); } + + Y_UNIT_TEST(ShouldAbortCompactionIfReadBlobFailsWithDeadlineExceeded) + { + NProto::TStorageServiceConfig config; + config.SetBlobStorageAsyncGetTimeoutHDD(TDuration::Seconds(1).MilliSeconds()); + auto runtime = PrepareTestActorRuntime(config); + + TPartitionClient partition(*runtime); + partition.WaitReady(); + + partition.WriteBlocks(3, 33); + partition.Flush(); + + ui32 failedReadBlob = 0; + runtime->SetEventFilter([&] + (TTestActorRuntimeBase& runtime, TAutoPtr& ev) + { + Y_UNUSED(runtime); + + if (ev->GetTypeRewrite() == TEvBlobStorage::EvVGet) { + const auto* msg = ev->Get(); + if (msg->Record.GetHandleClass() == NKikimrBlobStorage::AsyncRead && + msg->Record.GetMsgQoS().HasDeadlineSeconds()) + { + return true; + } + } else if (ev->GetTypeRewrite() == TEvStatsService::EvVolumePartCounters) { + auto* msg = + ev->Get(); + failedReadBlob = + msg->DiskCounters->Simple.ReadBlobDeadlineCount.Value; + } + return false; + }); + + partition.SendCompactionRequest(); + runtime->AdvanceCurrentTime(TDuration::Seconds(1)); + auto response = partition.RecvCompactionResponse(); + UNIT_ASSERT_VALUES_EQUAL(E_REJECTED, response->GetError().GetCode()); + + runtime->AdvanceCurrentTime(TDuration::Seconds(15)); + + partition.SendToPipe( + std::make_unique()); + { + TDispatchOptions options; + options.FinalEvents.emplace_back(TEvStatsService::EvVolumePartCounters); + runtime->DispatchEvents(options); + } + UNIT_ASSERT_VALUES_EQUAL(1, failedReadBlob); + } } } // namespace NCloud::NBlockStore::NStorage::NPartition2 diff --git a/cloud/blockstore/libs/storage/partition2/ya.make b/cloud/blockstore/libs/storage/partition2/ya.make index 63fb42fb752..efb6fe9a418 100644 --- a/cloud/blockstore/libs/storage/partition2/ya.make +++ b/cloud/blockstore/libs/storage/partition2/ya.make @@ -55,19 +55,20 @@ PEERDIR( cloud/blockstore/libs/storage/core cloud/blockstore/libs/storage/partition2/model cloud/blockstore/libs/storage/protos - + cloud/storage/core/libs/api cloud/storage/core/libs/common cloud/storage/core/libs/tablet - + library/cpp/cgiparam library/cpp/containers/dense_hash library/cpp/containers/intrusive_rb_tree library/cpp/containers/stack_vector library/cpp/lwtrace library/cpp/monlib/service/pages - + contrib/ydb/core/base + contrib/ydb/core/blobstorage contrib/ydb/core/node_whiteboard contrib/ydb/core/scheme contrib/ydb/core/tablet diff --git a/cloud/blockstore/libs/storage/partition_common/actor_read_blob.cpp b/cloud/blockstore/libs/storage/partition_common/actor_read_blob.cpp index d59d4875c3b..d9a0745969f 100644 --- a/cloud/blockstore/libs/storage/partition_common/actor_read_blob.cpp +++ b/cloud/blockstore/libs/storage/partition_common/actor_read_blob.cpp @@ -105,6 +105,10 @@ void TReadBlobActor::NotifyCompleted( request->RequestTime = ResponseReceived - RequestSent; request->GroupId = Request->GroupId; + if (DeadlineSeen) { + request->DeadlineSeen = true; + } + NCloud::Send(ctx, PartitionActorId, std::move(request)); } @@ -145,6 +149,10 @@ void TReadBlobActor::ReplyError( description.data(), response.Print(false).data()); + if (response.Status == NKikimrProto::DEADLINE) { + DeadlineSeen = true; + } + auto error = MakeError(E_REJECTED, "TEvBlobStorage::TEvGet failed: " + description); ReplyAndDie(ctx, std::make_unique(error)); } diff --git a/cloud/blockstore/libs/storage/partition_common/actor_read_blob.h b/cloud/blockstore/libs/storage/partition_common/actor_read_blob.h index 5782add7a19..3e99915ffe6 100644 --- a/cloud/blockstore/libs/storage/partition_common/actor_read_blob.h +++ b/cloud/blockstore/libs/storage/partition_common/actor_read_blob.h @@ -35,6 +35,8 @@ class TReadBlobActor final TInstant RequestSent; TInstant ResponseReceived; + bool DeadlineSeen = false; + public: TReadBlobActor( TRequestInfoPtr requestInfo, diff --git a/cloud/blockstore/libs/storage/partition_common/events_private.h b/cloud/blockstore/libs/storage/partition_common/events_private.h index 0deb020d756..da2be489308 100644 --- a/cloud/blockstore/libs/storage/partition_common/events_private.h +++ b/cloud/blockstore/libs/storage/partition_common/events_private.h @@ -110,6 +110,7 @@ struct TEvPartitionCommonPrivate ui32 BytesCount = 0; TDuration RequestTime; ui32 GroupId = 0; + bool DeadlineSeen = false; TReadBlobCompleted() = default;