From a5da82c0d357310977f8528df52912fdff7d848f Mon Sep 17 00:00:00 2001 From: yegorskii <137099343+yegorskii@users.noreply.github.com> Date: Wed, 7 Aug 2024 23:35:31 +0200 Subject: [PATCH] return retriable error when describe returns zero tablet id (#1737) (#1745) * return retriable error when describe returns zero tablet id * fix comment * update --------- Co-authored-by: yegorskii --- .../ss_proxy_actor_describevolume.cpp | 44 +++++++-- .../libs/storage/ss_proxy/ss_proxy_ut.cpp | 92 +++++++++++++++++++ .../ss_proxy/ss_proxy_actor_describefs.cpp | 29 ++++++ .../libs/storage/ss_proxy/ss_proxy_ut.cpp | 68 ++++++++++++++ 4 files changed, 227 insertions(+), 6 deletions(-) diff --git a/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_actor_describevolume.cpp b/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_actor_describevolume.cpp index e9854168ea6..a5d32b141d2 100644 --- a/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_actor_describevolume.cpp +++ b/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_actor_describevolume.cpp @@ -175,18 +175,50 @@ void TDescribeVolumeActor::HandleDescribeSchemeResponse( ctx, std::make_unique( MakeError( - E_FAIL, + E_INVALID_STATE, TStringBuilder() << "Described path is not a blockstore volume: " - << GetFullPath().Quote() - ), - GetFullPath() - ) - ); + << GetFullPath().Quote()), + GetFullPath())); return; } const auto& volumeDescription = pathDescription.GetBlockStoreVolumeDescription(); + + const auto& descr = msg->PathDescription.GetBlockStoreVolumeDescription(); + // Emptiness of VolumeTabletId or any of partition tablet ids means that + // blockstore volume is not configured by Hive yet. + + if (!descr.GetVolumeTabletId()) { + ReplyAndDie( + ctx, + std::make_unique( + MakeError( + E_REJECTED, + TStringBuilder() + << "Blockstore volume " << GetFullPath().Quote() + << " has zero VolumeTabletId"), + GetFullPath())); + return; + } + + for (const auto& part: descr.GetPartitions()) { + if (!part.GetTabletId()) { + auto error = MakeError( + E_REJECTED, + TStringBuilder() + << "Blockstore volume " << GetFullPath().Quote() + << " has zero TabletId for partition: " + << part.GetPartitionId()); + ReplyAndDie( + ctx, + std::make_unique( + std::move(error), + GetFullPath())); + return; + } + } + const auto& volumeConfig = volumeDescription.GetVolumeConfig(); if (!CheckVolume(ctx, volumeConfig)) { diff --git a/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_ut.cpp b/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_ut.cpp index dad09d35904..ebdf7710067 100644 --- a/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_ut.cpp +++ b/cloud/blockstore/libs/storage/ss_proxy/ss_proxy_ut.cpp @@ -1449,6 +1449,98 @@ Y_UNIT_TEST_SUITE(TSSProxyTest) response->GetError().GetMessage()); } } + + Y_UNIT_TEST(ShouldReturnERejectedIfVolumeTabletIdIsZero) + { + TTestEnv env; + SetupTestEnv(env); + auto& runtime = env.GetRuntime(); + + CreateVolume(runtime, "vol0", 4096); + + runtime.SetEventFilter([&] (auto& runtime, auto& ev) { + Y_UNUSED(runtime); + switch (ev->GetTypeRewrite()) { + case TEvSSProxy::EvDescribeSchemeResponse: { + using TEvent = TEvSSProxy::TEvDescribeSchemeResponse; + using TDescription = NKikimrSchemeOp::TPathDescription; + auto* msg = + ev->template Get(); + TDescription& desc = + const_cast(msg->PathDescription); + desc. + MutableBlockStoreVolumeDescription()-> + SetVolumeTabletId(0); + } + } + return false; + } + ); + + TActorId sender = runtime.AllocateEdgeActor(); + + Send( + runtime, + MakeSSProxyServiceId(), + sender, + std::make_unique("vol0")); + + TAutoPtr handle; + auto* response = + runtime.GrabEdgeEventRethrow( + handle); + + UNIT_ASSERT_VALUES_EQUAL(E_REJECTED, response->GetStatus()); + } + + Y_UNIT_TEST(ShouldReturnERejectedIfAnyOfPartitionIdsIsZero) + { + TTestEnv env; + SetupTestEnv(env); + auto& runtime = env.GetRuntime(); + + CreateVolume(runtime, "vol0", 4096); + + runtime.SetEventFilter([&] (auto& runtime, auto& ev) { + Y_UNUSED(runtime); + switch (ev->GetTypeRewrite()) { + case TEvSSProxy::EvDescribeSchemeResponse: { + using TEvent = TEvSSProxy::TEvDescribeSchemeResponse; + using TDescription = NKikimrSchemeOp::TPathDescription; + auto* msg = + ev->template Get(); + auto& desc = + const_cast(msg->PathDescription); + if (FAILED(msg->GetStatus()) || + desc.GetSelf().GetPathType() != + NKikimrSchemeOp::EPathTypeBlockStoreVolume) + { + break; + } + desc. + MutableBlockStoreVolumeDescription()-> + MutablePartitions()->at(0).SetTabletId(0); + } + } + return false; + } + ); + + TActorId sender = runtime.AllocateEdgeActor(); + + Send( + runtime, + MakeSSProxyServiceId(), + sender, + std::make_unique("vol0")); + + TAutoPtr handle; + auto* response = + runtime.GrabEdgeEventRethrow( + handle); + + UNIT_ASSERT_VALUES_EQUAL(E_REJECTED, response->GetStatus()); + } } } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/filestore/libs/storage/ss_proxy/ss_proxy_actor_describefs.cpp b/cloud/filestore/libs/storage/ss_proxy/ss_proxy_actor_describefs.cpp index 4746d41b5b8..7680c1b188b 100644 --- a/cloud/filestore/libs/storage/ss_proxy/ss_proxy_actor_describefs.cpp +++ b/cloud/filestore/libs/storage/ss_proxy/ss_proxy_actor_describefs.cpp @@ -97,6 +97,35 @@ void TDescribeFileStoreActor::HandleDescribeSchemeResponse( return; } + const auto pathType = msg->PathDescription.GetSelf().GetPathType(); + + auto path = GetFileSystemPath( + Config->GetSchemeShardDir(), + FileSystemId); + + if (pathType != NKikimrSchemeOp::EPathTypeFileStore) { + ReplyAndDie( + ctx, + MakeError( + E_INVALID_STATE, + TStringBuilder() + << "Described path is not a filestore: " + << path.Quote())); + return; + } + + // Zero IndexTabletId means that tablet is not configured by Hive yet. + if (!msg->PathDescription.GetFileStoreDescription().GetIndexTabletId()) { + ReplyAndDie( + ctx, + MakeError( + E_REJECTED, + TStringBuilder() + << "Filestore volume " << path.Quote() + << " has zero IndexTabletId")); + return; + } + auto response = std::make_unique( msg->Path, msg->PathDescription); diff --git a/cloud/filestore/libs/storage/ss_proxy/ss_proxy_ut.cpp b/cloud/filestore/libs/storage/ss_proxy/ss_proxy_ut.cpp index f2aefd9acb4..619570130e3 100644 --- a/cloud/filestore/libs/storage/ss_proxy/ss_proxy_ut.cpp +++ b/cloud/filestore/libs/storage/ss_proxy/ss_proxy_ut.cpp @@ -280,6 +280,74 @@ Y_UNIT_TEST_SUITE(TSSProxyTest) response->GetErrorReason() ); } + + Y_UNIT_TEST(ShouldReturnERejectedIfIndexTabletIdIsZero) + { + TTestEnv env; + env.CreateSubDomain("nfs"); + + ui32 nodeIdx = env.CreateNode("nfs"); + + TSSProxyClient ssProxy(env.GetStorageConfig(), env.GetRuntime(), nodeIdx); + ssProxy.CreateFileStore("test", 2000); + + auto& runtime = env.GetRuntime(); + runtime.SetEventFilter([&] (auto& runtime, auto& ev) { + Y_UNUSED(runtime); + switch (ev->GetTypeRewrite()) { + case TEvSSProxy::EvDescribeSchemeResponse: { + using TEvent = TEvSSProxy::TEvDescribeSchemeResponse; + using TDescription = NKikimrSchemeOp::TPathDescription; + auto* msg = ev->template Get(); + auto& desc = + const_cast(msg->PathDescription); + desc. + MutableFileStoreDescription()-> + SetIndexTabletId(0); + } + } + return false; + } + ); + + ssProxy.SendDescribeFileStoreRequest("test"); + auto describe = ssProxy.RecvDescribeFileStoreResponse(); + UNIT_ASSERT_VALUES_EQUAL(E_REJECTED, describe->GetStatus()); + } + + Y_UNIT_TEST(ShouldFailRequestIfWrongPathTypeIsReturnedFromSS) + { + TTestEnv env; + env.CreateSubDomain("nfs"); + + ui32 nodeIdx = env.CreateNode("nfs"); + + TSSProxyClient ssProxy(env.GetStorageConfig(), env.GetRuntime(), nodeIdx); + ssProxy.CreateFileStore("test", 2000); + + auto& runtime = env.GetRuntime(); + runtime.SetEventFilter([&] (auto& runtime, auto& ev) { + Y_UNUSED(runtime); + switch (ev->GetTypeRewrite()) { + case TEvSSProxy::EvDescribeSchemeResponse: { + using TEvent = TEvSSProxy::TEvDescribeSchemeResponse; + using TDescription = NKikimrSchemeOp::TPathDescription; + auto* msg = ev->template Get(); + auto& desc = + const_cast(msg->PathDescription); + desc. + MutableSelf()-> + SetPathType(NKikimrSchemeOp::EPathTypeBlockStoreVolume); + } + } + return false; + } + ); + + ssProxy.SendDescribeFileStoreRequest("test"); + auto describe = ssProxy.RecvDescribeFileStoreResponse(); + UNIT_ASSERT_VALUES_EQUAL(E_INVALID_STATE, describe->GetStatus()); + } } } // namespace NCloud::NFileStore::NStorage