issue-2674: cross-shard RenameNode implementation - fixed move to roo…

…t, fixed shard id selection check for dirs
ydb-platform · Jan 14, 2025 · 785be8b · 785be8b
1 parent 83fa5e2
commit 785be8b
Show file tree

Hide file tree

Showing 10 changed files with 64 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -461,14 +461,17 @@ bool TIndexTabletActor::PrepareTx_CreateNode(
 {
     Y_UNUSED(ctx);
 
+    const bool isMainWithLocalNodes =
+        IsMainTablet() && GetLastNodeId() > RootNodeId;
+
     if (!BehaveAsShard(args.Request.GetHeaders())
             && Config->GetShardIdSelectionInLeaderEnabled()
             && !GetFileSystem().GetShardFileSystemIds().empty()
             && (args.Attrs.GetType() == NProto::E_REGULAR_NODE
                 || 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
@@ -92,26 +92,39 @@ void TIndexTabletActor::HandleRenameNode(
 
         const auto newParentShardNo =
             ExtractShardNo(msg->Record.GetNewParentId());
-        if (newParentShardNo > static_cast<ui32>(shardIds.size())
-                || newParentShardNo == 0)
-        {
-            auto message = ReportInvalidShardNo(
-                TStringBuilder() << "RenameNode: "
-                    << msg->Record.ShortDebugString() << " newParentShardNo: "
-                    << newParentShardNo << ", shard count: "
-                    << shardIds.size());
-            auto response = std::make_unique<TEvService::TEvRenameNodeResponse>(
-                MakeError(E_ARGUMENT, std::move(message)));
-            NCloud::Reply(ctx, *requestInfo, std::move(response));
-            return;
-        }
-
         if (newParentShardNo != GetFileSystem().GetShardNo()) {
-            ExecuteTx<TPrepareRenameNodeInSource>(
-                ctx,
-                std::move(requestInfo),
-                std::move(msg->Record),
-                shardIds[newParentShardNo - 1]);
+            if (newParentShardNo > static_cast<ui32>(shardIds.size())) {
+                auto message = ReportInvalidShardNo(
+                    TStringBuilder() << "RenameNode: "
+                        << msg->Record.ShortDebugString() << " newParentShardNo"
+                        << ": " << newParentShardNo << ", shard count: "
+                        << shardIds.size());
+                auto response =
+                    std::make_unique<TEvService::TEvRenameNodeResponse>(
+                        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<TEvService::TEvRenameNodeResponse>(
+                        MakeError(E_ARGUMENT, std::move(message)));
+                NCloud::Reply(ctx, *requestInfo, std::move(response));
+            } else {
+                ExecuteTx<TPrepareRenameNodeInSource>(
+                    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
@@ -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
@@ -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
@@ -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/...lestore/tests/client_sharded_dir/canondata/test.test_nonsharded_vs_sharded_fs/results.txt b/...lestore/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"
     }
 ][22;31m-	LINK: [0m[0;39m/does/not/matter/2
 [22;32m+	LINK: [0m[0;39m/does/not/matter/3
diff --git 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")