From 1af6e2b146036a1123fee540b6f593950e92c627 Mon Sep 17 00:00:00 2001 From: Anton Myagkov Date: Tue, 30 Jul 2024 13:16:21 +0200 Subject: [PATCH] issue-1384: destruction of an already destroyed fs should return S_FALSE (#1659) Problem: DestroyFileStore for an already destroyed fs returns NKikimrScheme::EStatus::StatusPathDoesNotExist instead of S_FALSE Bug was introduced in #1441 because DestroyFileStore actor and DescribeSessions actor are handling missing fs in different ways. --- .../service/service_actor_destroyfs.cpp | 10 +++++ .../libs/storage/service/service_ut.cpp | 45 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/cloud/filestore/libs/storage/service/service_actor_destroyfs.cpp b/cloud/filestore/libs/storage/service/service_actor_destroyfs.cpp index 8680a1a96d..4fabeecf56 100644 --- a/cloud/filestore/libs/storage/service/service_actor_destroyfs.cpp +++ b/cloud/filestore/libs/storage/service/service_actor_destroyfs.cpp @@ -97,6 +97,16 @@ void TDestroyFileStoreActor::HandleDescribeSessionsResponse( { const auto* msg = ev->Get(); if (HasError(msg->GetError())) { + if (msg->GetStatus() == + MAKE_SCHEMESHARD_ERROR( + NKikimrScheme::EStatus::StatusPathDoesNotExist)) + { + ReplyAndDie( + ctx, + MakeError(S_FALSE, FileSystemId.Quote() + " does not exist")); + return; + } + ReplyAndDie(ctx, msg->GetError()); return; } diff --git a/cloud/filestore/libs/storage/service/service_ut.cpp b/cloud/filestore/libs/storage/service/service_ut.cpp index b6be6a4d2e..b9d9721818 100644 --- a/cloud/filestore/libs/storage/service/service_ut.cpp +++ b/cloud/filestore/libs/storage/service/service_ut.cpp @@ -3448,6 +3448,51 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) destroyFileStoreResponse->GetErrorReason()); } + Y_UNIT_TEST(DestroyDestroyedFileStoreShouldFail) + { + TTestEnv env; + env.CreateSubDomain("nfs"); + + ui32 nodeIdx = env.CreateNode("nfs"); + + const TString fsId = "test"; + const TString fsId2 = "test2"; + const auto initialBlockCount = 1'000; + TServiceClient service(env.GetRuntime(), nodeIdx); + service.CreateFileStore(fsId, initialBlockCount); + service.CreateFileStore(fsId2, initialBlockCount); + + { + auto destroyFileStoreResponse = service.DestroyFileStore(fsId); + UNIT_ASSERT_VALUES_EQUAL_C( + S_OK, + destroyFileStoreResponse->GetStatus(), + destroyFileStoreResponse->GetErrorReason()); + + auto alreadyDestroyedFileStoreResponse = + service.DestroyFileStore(fsId); + UNIT_ASSERT_VALUES_EQUAL_C( + S_FALSE, + alreadyDestroyedFileStoreResponse->GetStatus(), + alreadyDestroyedFileStoreResponse->GetErrorReason()); + } + + { + auto destroyFileStoreResponse = service.DestroyFileStore(fsId2); + UNIT_ASSERT_VALUES_EQUAL_C( + S_OK, + destroyFileStoreResponse->GetStatus(), + destroyFileStoreResponse->GetErrorReason()); + + auto alreadyDestroyedFileStoreResponse = + service.DestroyFileStore(fsId); + UNIT_ASSERT_VALUES_EQUAL_C( + S_FALSE, + alreadyDestroyedFileStoreResponse->GetStatus(), + alreadyDestroyedFileStoreResponse->GetErrorReason()); + } + } + Y_UNIT_TEST(ShouldValidateRequestsWithFollowerId) { TTestEnv env;