From 785be8b2355f1092e0e4f986ff74f61759c2d06d Mon Sep 17 00:00:00 2001 From: qkrorlqr Date: Tue, 14 Jan 2025 12:03:35 +0100 Subject: [PATCH] issue-2674: cross-shard RenameNode implementation - fixed move to root, fixed shard id selection check for dirs --- .../storage/service/service_actor_alterfs.cpp | 1 + .../service/service_actor_createfs.cpp | 1 + .../libs/storage/tablet/protos/tablet.proto | 1 + .../tablet/tablet_actor_createnode.cpp | 5 +- .../tablet/tablet_actor_renamenode.cpp | 51 ++++++++++++------- .../tablet/tablet_actor_updateconfig.cpp | 2 + .../libs/storage/tablet/tablet_state.h | 5 ++ .../filestore/private/api/protos/tablet.proto | 4 +- .../results.txt | 11 ++++ .../tests/client_sharded_dir/test.py | 4 ++ 10 files changed, 64 insertions(+), 21 deletions(-) diff --git a/cloud/filestore/libs/storage/service/service_actor_alterfs.cpp b/cloud/filestore/libs/storage/service/service_actor_alterfs.cpp index 05e2bb2624..59c659c1bd 100644 --- a/cloud/filestore/libs/storage/service/service_actor_alterfs.cpp +++ b/cloud/filestore/libs/storage/service/service_actor_alterfs.cpp @@ -416,6 +416,7 @@ void TAlterFileStoreActor::ConfigureShards(const TActorContext& ctx) request->Record.SetFileSystemId( FileStoreConfig.ShardConfigs[i].GetFileSystemId()); request->Record.SetShardNo(i + 1); + request->Record.SetMainFileSystemId(FileSystemId); if (StorageConfig->GetDirectoryCreationInShardsEnabled()) { for (const auto& shard: FileStoreConfig.ShardConfigs) { request->Record.AddShardFileSystemIds(shard.GetFileSystemId()); diff --git a/cloud/filestore/libs/storage/service/service_actor_createfs.cpp b/cloud/filestore/libs/storage/service/service_actor_createfs.cpp index 5c953c2f30..e4ffdf5e25 100644 --- a/cloud/filestore/libs/storage/service/service_actor_createfs.cpp +++ b/cloud/filestore/libs/storage/service/service_actor_createfs.cpp @@ -207,6 +207,7 @@ void TCreateFileStoreActor::ConfigureShards(const TActorContext& ctx) request->Record.SetFileSystemId( FileStoreConfig.ShardConfigs[i].GetFileSystemId()); request->Record.SetShardNo(i + 1); + request->Record.SetMainFileSystemId(Request.GetFileSystemId()); if (StorageConfig->GetDirectoryCreationInShardsEnabled()) { for (const auto& shard: FileStoreConfig.ShardConfigs) { request->Record.AddShardFileSystemIds(shard.GetFileSystemId()); diff --git a/cloud/filestore/libs/storage/tablet/protos/tablet.proto b/cloud/filestore/libs/storage/tablet/protos/tablet.proto index 4317b5438b..35f9bdaa87 100644 --- a/cloud/filestore/libs/storage/tablet/protos/tablet.proto +++ b/cloud/filestore/libs/storage/tablet/protos/tablet.proto @@ -47,6 +47,7 @@ message TFileSystem uint32 ShardNo = 14; bool AutomaticShardCreationEnabled = 15; uint64 ShardAllocationUnit = 16; + string MainFileSystemId = 17; } //////////////////////////////////////////////////////////////////////////////// diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp index 7c0d2912ab..a2a391d25e 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp +++ b/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp @@ -461,6 +461,9 @@ bool TIndexTabletActor::PrepareTx_CreateNode( { Y_UNUSED(ctx); + const bool isMainWithLocalNodes = + IsMainTablet() && GetLastNodeId() > RootNodeId; + if (!BehaveAsShard(args.Request.GetHeaders()) && Config->GetShardIdSelectionInLeaderEnabled() && !GetFileSystem().GetShardFileSystemIds().empty() @@ -468,7 +471,7 @@ bool TIndexTabletActor::PrepareTx_CreateNode( || Config->GetDirectoryCreationInShardsEnabled() // otherwise there might be some local nodes which breaks // current cross-shard RenameNode implementation - && GetLastNodeId() == RootNodeId)) + && !isMainWithLocalNodes)) { args.Error = SelectShard(args.Attrs.GetSize(), &args.ShardId); if (HasError(args.Error)) { diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_renamenode.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_renamenode.cpp index 51e3654f02..fb8be853e0 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_actor_renamenode.cpp +++ b/cloud/filestore/libs/storage/tablet/tablet_actor_renamenode.cpp @@ -92,26 +92,39 @@ void TIndexTabletActor::HandleRenameNode( const auto newParentShardNo = ExtractShardNo(msg->Record.GetNewParentId()); - if (newParentShardNo > static_cast(shardIds.size()) - || newParentShardNo == 0) - { - auto message = ReportInvalidShardNo( - TStringBuilder() << "RenameNode: " - << msg->Record.ShortDebugString() << " newParentShardNo: " - << newParentShardNo << ", shard count: " - << shardIds.size()); - auto response = std::make_unique( - MakeError(E_ARGUMENT, std::move(message))); - NCloud::Reply(ctx, *requestInfo, std::move(response)); - return; - } - if (newParentShardNo != GetFileSystem().GetShardNo()) { - ExecuteTx( - ctx, - std::move(requestInfo), - std::move(msg->Record), - shardIds[newParentShardNo - 1]); + if (newParentShardNo > static_cast(shardIds.size())) { + auto message = ReportInvalidShardNo( + TStringBuilder() << "RenameNode: " + << msg->Record.ShortDebugString() << " newParentShardNo" + << ": " << newParentShardNo << ", shard count: " + << shardIds.size()); + auto response = + std::make_unique( + MakeError(E_ARGUMENT, std::move(message))); + NCloud::Reply(ctx, *requestInfo, std::move(response)); + } else if (newParentShardNo == 0 + && msg->Record.GetNewParentId() != RootNodeId) + { + auto message = ReportInvalidShardNo( + TStringBuilder() << "RenameNode: " + << msg->Record.ShortDebugString() << " newParentShardNo" + << ": " << newParentShardNo << ", NewParentId: " + << msg->Record.GetNewParentId()); + auto response = + std::make_unique( + MakeError(E_ARGUMENT, std::move(message))); + NCloud::Reply(ctx, *requestInfo, std::move(response)); + } else { + ExecuteTx( + ctx, + std::move(requestInfo), + std::move(msg->Record), + newParentShardNo + ? shardIds[newParentShardNo - 1] + : GetMainFileSystemId()); + } + return; } } diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_updateconfig.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_updateconfig.cpp index 791e9b4c91..74bcaf7462 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_actor_updateconfig.cpp +++ b/cloud/filestore/libs/storage/tablet/tablet_actor_updateconfig.cpp @@ -158,6 +158,7 @@ void TIndexTabletActor::HandleUpdateConfig( *newConfig.MutableShardFileSystemIds() = oldConfig.GetShardFileSystemIds(); newConfig.SetShardNo(oldConfig.GetShardNo()); + newConfig.SetMainFileSystemId(oldConfig.GetMainFileSystemId()); newConfig.SetAutomaticShardCreationEnabled( oldConfig.GetAutomaticShardCreationEnabled()); newConfig.SetShardAllocationUnit(oldConfig.GetShardAllocationUnit()); @@ -430,6 +431,7 @@ void TIndexTabletActor::ExecuteTx_ConfigureAsShard( auto config = GetFileSystem(); config.SetShardNo(args.Request.GetShardNo()); + config.SetMainFileSystemId(args.Request.GetMainFileSystemId()); *config.MutableShardFileSystemIds() = std::move(*args.Request.MutableShardFileSystemIds()); diff --git a/cloud/filestore/libs/storage/tablet/tablet_state.h b/cloud/filestore/libs/storage/tablet/tablet_state.h index e6360158e8..7d708abf43 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_state.h +++ b/cloud/filestore/libs/storage/tablet/tablet_state.h @@ -230,6 +230,11 @@ class TIndexTabletState return FileSystem.GetFileSystemId(); } + TString GetMainFileSystemId() const + { + return FileSystem.GetMainFileSystemId(); + } + ui32 GetGeneration() const { return Generation; diff --git a/cloud/filestore/private/api/protos/tablet.proto b/cloud/filestore/private/api/protos/tablet.proto index 07cf14efc9..6ece88e4ac 100644 --- a/cloud/filestore/private/api/protos/tablet.proto +++ b/cloud/filestore/private/api/protos/tablet.proto @@ -596,8 +596,10 @@ message TConfigureAsShardRequest // ShardNo (will be used for the high bits of NodeId and HandleId) uint32 ShardNo = 2; - // Shard FileSystem identifiers. Needed for directory creation in shards. + // Shard FileSystem identifiers and main FileSystem identifier (manages + // RootNode). Needed for directory creation in shards. repeated string ShardFileSystemIds = 3; + string MainFileSystemId = 4; } message TConfigureAsShardResponse diff --git a/cloud/filestore/tests/client_sharded_dir/canondata/test.test_nonsharded_vs_sharded_fs/results.txt b/cloud/filestore/tests/client_sharded_dir/canondata/test.test_nonsharded_vs_sharded_fs/results.txt index 434759e131..f76b6b66e2 100644 --- a/cloud/filestore/tests/client_sharded_dir/canondata/test.test_nonsharded_vs_sharded_fs/results.txt +++ b/cloud/filestore/tests/client_sharded_dir/canondata/test.test_nonsharded_vs_sharded_fs/results.txt @@ -12,6 +12,13 @@ "Name": "a1", "Mode": 511, "Id": 17 + }, + { + "Type": 1, + "Links": 1, + "Size": 62, + "Name": "f18.txt", + "Id": 29 } ][ { @@ -21,6 +28,10 @@ { "ShardFileSystemId": "masked_for_test_stability", "Name": "a1" + }, + { + "ShardFileSystemId": "masked_for_test_stability", + "Name": "f18.txt" } ]- LINK: /does/not/matter/2 + LINK: /does/not/matter/3 diff --git a/cloud/filestore/tests/client_sharded_dir/test.py b/cloud/filestore/tests/client_sharded_dir/test.py index c268190741..4aa96beb06 100644 --- a/cloud/filestore/tests/client_sharded_dir/test.py +++ b/cloud/filestore/tests/client_sharded_dir/test.py @@ -130,6 +130,7 @@ def _l(path, symlink): _l("/a1/b2/l1", "/does/not/matter"), _f("/a1/b2/f16.txt", "ZZZZZZZZZZZ2"), _f("/f17.txt", "010101010101010101"), + _f("/a1/f18.txt", "xxxxxxxxxxxxxxxxxxxxxxxxxx"), _d("/a1/b3"), ] @@ -154,6 +155,9 @@ def _l(path, symlink): client.mv("fs1", "/a0/b0/c0/f17.txt", "/a0/b0/c0/d0/f17.txt") client.mv("fs1", "/a0/b0/c0/d0/f17.txt", "/a1/f17.txt") client.mv("fs1", "/a1/f17.txt", "/a1/b2/f17.txt") + # checking that cross-directory mv to root works + client.mv("fs0", "/a1/f18.txt", "/f18.txt") + client.mv("fs1", "/a1/f18.txt", "/f18.txt") # checking that readlink works (indirectly - via diff) client.ln("fs0", "/a1/b2/l2", "--symlink", "/does/not/matter/2") client.ln("fs1", "/a1/b2/l2", "--symlink", "/does/not/matter/3")