From 19f33516eec1869c3b384edcfbb7bc5734257b66 Mon Sep 17 00:00:00 2001 From: yegorskii Date: Wed, 30 Oct 2024 17:58:15 +0100 Subject: [PATCH] issue-2376: implement special switches to disable TwoStageReads and ThreeStageWrites for hdd fs (#2377) * issue-2376: implement special switches to disable TwoStageReads and ThreeStageWrites for hdd fs * update * update --- cloud/filestore/config/storage.proto | 6 + cloud/filestore/libs/storage/core/config.cpp | 2 + cloud/filestore/libs/storage/core/config.h | 3 + .../service/service_actor_readdata.cpp | 13 +- .../service/service_actor_writedata.cpp | 17 +- .../libs/storage/service/service_ut.cpp | 206 ++++++++++++++++-- .../tablet/tablet_actor_createsession.cpp | 5 +- .../libs/storage/testlib/test_env.cpp | 6 + cloud/filestore/public/api/protos/fs.proto | 2 + 9 files changed, 239 insertions(+), 21 deletions(-) diff --git a/cloud/filestore/config/storage.proto b/cloud/filestore/config/storage.proto index 7c31649c173..b5ab5e50a3f 100644 --- a/cloud/filestore/config/storage.proto +++ b/cloud/filestore/config/storage.proto @@ -450,4 +450,10 @@ message TStorageConfig // rewriting the data belonging to this range. optional uint32 CompactRangeGarbagePercentageThreshold = 401; optional uint32 CompactRangeAverageBlobSizeThreshold = 402; + + // Disables ThreeStageWrite for HDD filesystems. + optional bool ThreeStageWriteDisabledForHDD = 403; + + // Disables TwoStageRead for HDD filesystems. + optional bool TwoStageReadDisabledForHDD = 404; } diff --git a/cloud/filestore/libs/storage/core/config.cpp b/cloud/filestore/libs/storage/core/config.cpp index a81915db456..ba4999b20ad 100644 --- a/cloud/filestore/libs/storage/core/config.cpp +++ b/cloud/filestore/libs/storage/core/config.cpp @@ -160,8 +160,10 @@ using TAliases = NProto::TStorageConfig::TFilestoreAliases; NCloud::NProto::AUTHORIZATION_IGNORE )\ \ xxx(TwoStageReadEnabled, bool, false )\ + xxx(TwoStageReadDisabledForHDD, bool, false )\ xxx(ThreeStageWriteEnabled, bool, false )\ xxx(ThreeStageWriteThreshold, ui32, 64_KB )\ + xxx(ThreeStageWriteDisabledForHDD, bool, false )\ xxx(UnalignedThreeStageWriteEnabled, bool, false )\ xxx(ReadAheadCacheMaxNodes, ui32, 1024 )\ xxx(ReadAheadCacheMaxResultsPerNode, ui32, 32 )\ diff --git a/cloud/filestore/libs/storage/core/config.h b/cloud/filestore/libs/storage/core/config.h index 825d60c8f99..7181307d31a 100644 --- a/cloud/filestore/libs/storage/core/config.h +++ b/cloud/filestore/libs/storage/core/config.h @@ -283,6 +283,9 @@ class TStorageConfig TVector GetDestroyFilestoreDenyList() const; bool GetSSProxyFallbackMode() const; + + bool GetTwoStageReadDisabledForHDD() const; + bool GetThreeStageWriteDisabledForHDD() const; }; } // namespace NCloud::NFileStore::NStorage diff --git a/cloud/filestore/libs/storage/service/service_actor_readdata.cpp b/cloud/filestore/libs/storage/service/service_actor_readdata.cpp index 328a678df0e..b1820d10439 100644 --- a/cloud/filestore/libs/storage/service/service_actor_readdata.cpp +++ b/cloud/filestore/libs/storage/service/service_actor_readdata.cpp @@ -23,6 +23,17 @@ namespace { //////////////////////////////////////////////////////////////////////////////// +bool IsTwoStageReadEnabled(const NProto::TFileStore& fs) +{ + const auto isHdd = fs.GetStorageMediaKind() == NProto::STORAGE_MEDIA_HYBRID + || fs.GetStorageMediaKind() == NProto::STORAGE_MEDIA_HDD; + const auto disabledAsHdd = isHdd && + fs.GetFeatures().GetTwoStageReadDisabledForHDD(); + return !disabledAsHdd && fs.GetFeatures().GetTwoStageReadEnabled(); +} + +//////////////////////////////////////////////////////////////////////////////// + class TReadDataActor final: public TActorBootstrapped { private: @@ -647,7 +658,7 @@ void TStorageServiceActor::HandleReadData( msg->Record.SetFileSystemId(fsId); } - if (!filestore.GetFeatures().GetTwoStageReadEnabled()) { + if (!IsTwoStageReadEnabled(filestore)) { // If two-stage read is disabled, forward the request to the tablet in // the same way as all other requests. ForwardRequest(ctx, ev); diff --git a/cloud/filestore/libs/storage/service/service_actor_writedata.cpp b/cloud/filestore/libs/storage/service/service_actor_writedata.cpp index a72f0f8cc31..bba2441d52c 100644 --- a/cloud/filestore/libs/storage/service/service_actor_writedata.cpp +++ b/cloud/filestore/libs/storage/service/service_actor_writedata.cpp @@ -24,6 +24,17 @@ namespace { //////////////////////////////////////////////////////////////////////////////// +bool IsThreeStageWriteEnabled(const NProto::TFileStore& fs) +{ + const auto isHdd = fs.GetStorageMediaKind() == NProto::STORAGE_MEDIA_HYBRID + || fs.GetStorageMediaKind() == NProto::STORAGE_MEDIA_HDD; + const auto disabledAsHdd = isHdd && + fs.GetFeatures().GetThreeStageWriteDisabledForHDD(); + return !disabledAsHdd && fs.GetFeatures().GetThreeStageWriteEnabled(); +} + +//////////////////////////////////////////////////////////////////////////////// + class TWriteDataActor final: public TActorBootstrapped { private: @@ -478,7 +489,9 @@ void TStorageServiceActor::HandleWriteData( msg->Record.SetFileSystemId(fsId); } - if (!filestore.GetFeatures().GetThreeStageWriteEnabled()) { + const auto threeStageWriteAllowed = IsThreeStageWriteEnabled(filestore); + + if (!threeStageWriteAllowed) { // If three-stage write is disabled, forward the request to the tablet // in the same way as all other requests. ForwardRequest(ctx, ev); @@ -493,7 +506,7 @@ void TStorageServiceActor::HandleWriteData( blockSize); const bool threeStageWriteEnabled = range.Length >= filestore.GetFeatures().GetThreeStageWriteThreshold() - && filestore.GetFeatures().GetThreeStageWriteEnabled() + && threeStageWriteAllowed && (range.IsAligned() || StorageConfig->GetUnalignedThreeStageWriteEnabled()); if (threeStageWriteEnabled) { diff --git a/cloud/filestore/libs/storage/service/service_ut.cpp b/cloud/filestore/libs/storage/service/service_ut.cpp index b831f791a15..2d22793555b 100644 --- a/cloud/filestore/libs/storage/service/service_ut.cpp +++ b/cloud/filestore/libs/storage/service/service_ut.cpp @@ -1875,7 +1875,7 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) service.CreateSession(headers); } - Y_UNIT_TEST(ShouldPerformTwoStageReads) + void CheckTwoStageReads(NProto::EStorageMediaKind mediaKind, bool disableForHdd) { TTestEnv env; env.CreateSubDomain("nfs"); @@ -1884,16 +1884,21 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) TServiceClient service(env.GetRuntime(), nodeIdx); const TString fs = "test"; - service.CreateFileStore(fs, 1000); + service.CreateFileStore(fs, 1000, DefaultBlockSize, mediaKind); { NProto::TStorageConfig newConfig; newConfig.SetTwoStageReadEnabled(true); + newConfig.SetTwoStageReadDisabledForHDD(disableForHdd); const auto response = ExecuteChangeStorageConfig(std::move(newConfig), service); UNIT_ASSERT_VALUES_EQUAL( - response.GetStorageConfig().GetTwoStageReadEnabled(), - true); + true, + response.GetStorageConfig().GetTwoStageReadEnabled()); + UNIT_ASSERT_VALUES_EQUAL( + disableForHdd, + response.GetStorageConfig().GetTwoStageReadDisabledForHDD()); + TDispatchOptions options; env.GetRuntime().DispatchEvents(options, TDuration::Seconds(1)); } @@ -1916,7 +1921,7 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) service.WriteData(headers, fs, nodeId, handle, 0, data); auto readDataResult = service.ReadData(headers, fs, nodeId, handle, 0, data.size()); - UNIT_ASSERT_VALUES_EQUAL(readDataResult->Record.GetBuffer(), data); + UNIT_ASSERT_VALUES_EQUAL(data, readDataResult->Record.GetBuffer()); // fresh blocks - adding multiple adjacent blocks is important here to // catch some subtle bugs @@ -1924,14 +1929,14 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) service.WriteData(headers, fs, nodeId, handle, 0, data); readDataResult = service.ReadData(headers, fs, nodeId, handle, 0, data.size()); - UNIT_ASSERT_VALUES_EQUAL(readDataResult->Record.GetBuffer(), data); + UNIT_ASSERT_VALUES_EQUAL(data, readDataResult->Record.GetBuffer()); // blobs data = TString(1_MB, 'b'); service.WriteData(headers, fs, nodeId, handle, 0, data); readDataResult = service.ReadData(headers, fs, nodeId, handle, 0, data.size()); - UNIT_ASSERT_VALUES_EQUAL(readDataResult->Record.GetBuffer(), data); + UNIT_ASSERT_VALUES_EQUAL(data, readDataResult->Record.GetBuffer()); readDataResult = service.ReadData( headers, @@ -1941,8 +1946,8 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) DefaultBlockSize, data.size() - DefaultBlockSize); UNIT_ASSERT_VALUES_EQUAL( - readDataResult->Record.GetBuffer(), - data.substr(DefaultBlockSize)); + data.substr(DefaultBlockSize), + readDataResult->Record.GetBuffer()); // mix auto patch = TString(4_KB, 'c'); @@ -1951,7 +1956,7 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) readDataResult = service.ReadData(headers, fs, nodeId, handle, 0, data.size()); memcpy(data.begin() + patchOffset, patch.data(), patch.size()); - UNIT_ASSERT_VALUES_EQUAL(readDataResult->Record.GetBuffer(), data); + UNIT_ASSERT_VALUES_EQUAL(data, readDataResult->Record.GetBuffer()); auto counters = env.GetCounters() ->FindSubgroup("component", "service_fs") @@ -1982,6 +1987,21 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) } } + Y_UNIT_TEST(ShouldPerformTwoStageReadsHdd) + { + CheckTwoStageReads(NProto::STORAGE_MEDIA_HDD, false); + } + + Y_UNIT_TEST(ShouldPerformTwoStageReadsHybrid) + { + CheckTwoStageReads(NProto::STORAGE_MEDIA_HYBRID, false); + } + + Y_UNIT_TEST(ShouldPerformTwoStageReadsSsd) + { + CheckTwoStageReads(NProto::STORAGE_MEDIA_SSD, false); + } + Y_UNIT_TEST(ShouldFallbackToReadDataIfDescribeDataFails) { TTestEnv env; @@ -2212,7 +2232,7 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) return data; } - Y_UNIT_TEST(ShouldPerformThreeStageWrites) + void CheckThreeStageWrites(NProto::EStorageMediaKind kind, bool disableForHdd) { TTestEnv env; env.CreateSubDomain("nfs"); @@ -2221,20 +2241,25 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) TServiceClient service(env.GetRuntime(), nodeIdx); const TString fs = "test"; - service.CreateFileStore(fs, 1000); + service.CreateFileStore(fs, 1000, DefaultBlockSize, kind); { NProto::TStorageConfig newConfig; newConfig.SetThreeStageWriteEnabled(true); newConfig.SetThreeStageWriteThreshold(1); + newConfig.SetThreeStageWriteDisabledForHDD(disableForHdd); const auto response = ExecuteChangeStorageConfig(std::move(newConfig), service); UNIT_ASSERT_VALUES_EQUAL( - response.GetStorageConfig().GetThreeStageWriteEnabled(), - true); + true, + response.GetStorageConfig().GetThreeStageWriteEnabled()); + UNIT_ASSERT_VALUES_EQUAL( + 1, + response.GetStorageConfig().GetThreeStageWriteThreshold()); UNIT_ASSERT_VALUES_EQUAL( - response.GetStorageConfig().GetThreeStageWriteThreshold(), - 1); + disableForHdd, + response.GetStorageConfig().GetThreeStageWriteDisabledForHDD()); + TDispatchOptions options; env.GetRuntime().DispatchEvents(options, TDuration::Seconds(1)); } @@ -2291,7 +2316,7 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) service .ReadData(headers, fs, nodeId, handle, offset, data.size()); // clang-format off - UNIT_ASSERT_VALUES_EQUAL(readDataResult->Record.GetBuffer(), data); + UNIT_ASSERT_VALUES_EQUAL(data, readDataResult->Record.GetBuffer()); UNIT_ASSERT_VALUES_EQUAL(2, runtime.GetCounter(TEvIndexTablet::EvGenerateBlobIdsRequest)); UNIT_ASSERT_VALUES_EQUAL(2, runtime.GetCounter(TEvIndexTablet::EvAddDataRequest)); UNIT_ASSERT_VALUES_EQUAL(1, runtime.GetCounter(TEvIndexTabletPrivate::EvAddBlobRequest)); @@ -2368,6 +2393,21 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) } } + Y_UNIT_TEST(ShouldPerformThreeStageWritesHdd) + { + CheckThreeStageWrites(NProto::STORAGE_MEDIA_HDD, false); + } + + Y_UNIT_TEST(ShouldPerformThreeStageWritesSsd) + { + CheckThreeStageWrites(NProto::STORAGE_MEDIA_SSD, false); + } + + Y_UNIT_TEST(ShouldPerformThreeStageWritesHybrid) + { + CheckThreeStageWrites(NProto::STORAGE_MEDIA_HYBRID, false); + } + Y_UNIT_TEST(ShouldNotUseThreeStageWriteForSmallOrUnalignedRequests) { TTestEnv env; @@ -5805,6 +5845,138 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest) UNIT_ASSERT_VALUES_EQUAL(lastCompactionMapRangeId, 29); } + + void CheckDisableMultistageReadWritesForHdd(NProto::EStorageMediaKind kind) + { + TTestEnv env; + env.CreateSubDomain("nfs"); + + ui32 nodeIdx = env.CreateNode("nfs"); + + TServiceClient service(env.GetRuntime(), nodeIdx); + const TString fs = "test"; + service.CreateFileStore(fs, 1000, DefaultBlockSize, kind); + + { + NProto::TStorageConfig newConfig; + newConfig.SetTwoStageReadEnabled(true); + newConfig.SetThreeStageWriteEnabled(true); + newConfig.SetThreeStageWriteDisabledForHDD(true); + newConfig.SetThreeStageWriteThreshold(1); + newConfig.SetTwoStageReadDisabledForHDD(true); + + const auto response = + ExecuteChangeStorageConfig(std::move(newConfig), service); + + UNIT_ASSERT_VALUES_EQUAL( + true, + response.GetStorageConfig().GetThreeStageWriteEnabled()); + UNIT_ASSERT_VALUES_EQUAL( + true, + response.GetStorageConfig().GetThreeStageWriteDisabledForHDD()); + UNIT_ASSERT_VALUES_EQUAL( + 1, + response.GetStorageConfig().GetThreeStageWriteThreshold()); + UNIT_ASSERT_VALUES_EQUAL( + true, + response.GetStorageConfig().GetTwoStageReadEnabled()); + UNIT_ASSERT_VALUES_EQUAL( + true, + response.GetStorageConfig().GetTwoStageReadDisabledForHDD()); + + TDispatchOptions options; + env.GetRuntime().DispatchEvents(options, TDuration::Seconds(1)); + } + + auto headers = service.InitSession(fs, "client"); + ui64 nodeId = service + .CreateNode(headers, TCreateNodeArgs::File(RootNodeId, "file")) + ->Record.GetNode() + .GetId(); + ui64 handle = service + .CreateHandle(headers, fs, nodeId, "", TCreateHandleArgs::RDWR) + ->Record.GetHandle(); + + auto data = GenerateValidateData(2 * DefaultBlockSize); + service.WriteData(headers, fs, nodeId, handle, 0, data); + + auto readDataResult = + service + .ReadData(headers, fs, nodeId, handle, 0, data.size()); + + UNIT_ASSERT_VALUES_EQUAL(data, readDataResult->Record.GetBuffer()); + + auto counters = env.GetCounters() + ->FindSubgroup("component", "service_fs") + ->FindSubgroup("host", "cluster") + ->FindSubgroup("filesystem", fs) + ->FindSubgroup("client", "client"); + { + auto subgroup = counters->FindSubgroup("request", "GenerateBlobIds"); + UNIT_ASSERT(subgroup); + UNIT_ASSERT_VALUES_EQUAL( + 0, + subgroup->GetCounter("Count")->GetAtomic()); + } + { + auto subgroup = counters->FindSubgroup("request", "AddData"); + UNIT_ASSERT(subgroup); + UNIT_ASSERT_VALUES_EQUAL( + 0, + subgroup->GetCounter("Count")->GetAtomic()); + } + { + auto subgroup = counters->FindSubgroup("request", "WriteData"); + UNIT_ASSERT(subgroup); + UNIT_ASSERT_VALUES_EQUAL( + 1, + subgroup->GetCounter("Count")->GetAtomic()); + } + { + auto subgroup = counters->FindSubgroup("request", "WriteBlob"); + UNIT_ASSERT(subgroup); + UNIT_ASSERT_VALUES_EQUAL( + 0, + subgroup->GetCounter("Count")->GetAtomic()); + } + { + auto subgroup = counters->FindSubgroup("request", "DescribeData"); + UNIT_ASSERT(subgroup); + UNIT_ASSERT_VALUES_EQUAL( + 0, + subgroup->GetCounter("Count")->GetAtomic()); + } + { + auto subgroup = counters->FindSubgroup("request", "ReadBlob"); + UNIT_ASSERT(subgroup); + UNIT_ASSERT_VALUES_EQUAL( + 0, + subgroup->GetCounter("Count")->GetAtomic()); + } + { + auto subgroup = counters->FindSubgroup("request", "ReadData"); + UNIT_ASSERT(subgroup); + UNIT_ASSERT_VALUES_EQUAL( + 1, + subgroup->GetCounter("Count")->GetAtomic()); + } + } + + Y_UNIT_TEST(ShouldNotPerformThreeStageWritesAndTwoStageReadsForHddIfDisabled) + { + CheckDisableMultistageReadWritesForHdd(NProto::STORAGE_MEDIA_HDD); + } + + Y_UNIT_TEST(ShouldNotPerformThreeStageWritesAndTwoStageReadsForHybridIfDisabled) + { + CheckDisableMultistageReadWritesForHdd(NProto::STORAGE_MEDIA_HYBRID); + } + + Y_UNIT_TEST(ShouldNotAffectSsdReadWritesIfMultistageReadWritesAreOffForHdd) + { + CheckThreeStageWrites(NProto::STORAGE_MEDIA_SSD, true); + CheckTwoStageReads(NProto::STORAGE_MEDIA_SSD, true); + } } } // namespace NCloud::NFileStore::NStorage diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_createsession.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_createsession.cpp index 28dc13626ed..6e25a6104eb 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_actor_createsession.cpp +++ b/cloud/filestore/libs/storage/tablet/tablet_actor_createsession.cpp @@ -21,11 +21,14 @@ void FillFeatures(const TStorageConfig& config, NProto::TFileStore& fileStore) auto* features = fileStore.MutableFeatures(); features->SetTwoStageReadEnabled(config.GetTwoStageReadEnabled()); features->SetThreeStageWriteEnabled(config.GetThreeStageWriteEnabled()); + features->SetTwoStageReadDisabledForHDD( + config.GetTwoStageReadDisabledForHDD()); + features->SetThreeStageWriteDisabledForHDD( + config.GetThreeStageWriteDisabledForHDD()); features->SetEntryTimeout(config.GetEntryTimeout().MilliSeconds()); features->SetNegativeEntryTimeout( config.GetNegativeEntryTimeout().MilliSeconds()); features->SetAttrTimeout(config.GetAttrTimeout().MilliSeconds()); - features->SetThreeStageWriteEnabled(config.GetThreeStageWriteEnabled()); features->SetThreeStageWriteThreshold(config.GetThreeStageWriteThreshold()); auto preferredBlockSizeMultiplier = diff --git a/cloud/filestore/libs/storage/testlib/test_env.cpp b/cloud/filestore/libs/storage/testlib/test_env.cpp index 5240477b233..ed17813544b 100644 --- a/cloud/filestore/libs/storage/testlib/test_env.cpp +++ b/cloud/filestore/libs/storage/testlib/test_env.cpp @@ -899,6 +899,12 @@ TStorageConfigPtr CreateTestStorageConfig(NProto::TStorageConfig storageConfig) storageConfig.SetSSDFreshChannelPoolKind("pool-kind-2"); storageConfig.SetSSDMixedChannelPoolKind("pool-kind-2"); + storageConfig.SetHybridSystemChannelPoolKind("pool-kind-2"); + storageConfig.SetHybridLogChannelPoolKind("pool-kind-2"); + storageConfig.SetHybridIndexChannelPoolKind("pool-kind-2"); + storageConfig.SetHybridFreshChannelPoolKind("pool-kind-2"); + storageConfig.SetHybridMixedChannelPoolKind("pool-kind-1"); + return std::make_shared(storageConfig); } diff --git a/cloud/filestore/public/api/protos/fs.proto b/cloud/filestore/public/api/protos/fs.proto index 2cc4f33b991..85a4a68f505 100644 --- a/cloud/filestore/public/api/protos/fs.proto +++ b/cloud/filestore/public/api/protos/fs.proto @@ -24,6 +24,8 @@ message TFileStoreFeatures uint32 AsyncHandleOperationPeriod = 9; bool DirectIoEnabled = 10; uint32 DirectIoAlign = 11; + bool TwoStageReadDisabledForHDD = 12; + bool ThreeStageWriteDisabledForHDD = 13; } message TFileStore