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 <[email protected]>
ydb-platform · Aug 7, 2024 · a5da82c · a5da82c
1 parent 2a3cb15
commit a5da82c
Show file tree

Hide file tree

Showing 4 changed files with 227 additions and 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
@@ -175,18 +175,50 @@ void TDescribeVolumeActor::HandleDescribeSchemeResponse(
             ctx,
             std::make_unique<TEvSSProxy::TEvDescribeVolumeResponse>(
                 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<TEvSSProxy::TEvDescribeVolumeResponse>(
+                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<TEvSSProxy::TEvDescribeVolumeResponse>(
+                    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
@@ -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<TEvent>();
+                        TDescription& desc =
+                            const_cast<TDescription&>(msg->PathDescription);
+                        desc.
+                            MutableBlockStoreVolumeDescription()->
+                            SetVolumeTabletId(0);
+                    }
+                }
+                return false;
+            }
+        );
+
+        TActorId sender = runtime.AllocateEdgeActor();
+
+        Send(
+            runtime,
+            MakeSSProxyServiceId(),
+            sender,
+            std::make_unique<TEvSSProxy::TEvDescribeVolumeRequest>("vol0"));
+
+        TAutoPtr<IEventHandle> handle;
+        auto* response =
+            runtime.GrabEdgeEventRethrow<TEvSSProxy::TEvDescribeVolumeResponse>(
+                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<TEvent>();
+                        auto& desc =
+                            const_cast<TDescription&>(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<TEvSSProxy::TEvDescribeVolumeRequest>("vol0"));
+
+        TAutoPtr<IEventHandle> handle;
+        auto* response =
+            runtime.GrabEdgeEventRethrow<TEvSSProxy::TEvDescribeVolumeResponse>(
+                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
@@ -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<TEvSSProxy::TEvDescribeFileStoreResponse>(
         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
@@ -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<TEvent>();
+                        auto& desc =
+                            const_cast<TDescription&>(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<TEvent>();
+                        auto& desc =
+                            const_cast<TDescription&>(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