issue-1146: resetsession should update in-memory cache as it can modi…

…fy node index (#2525)
ydb-platform · Nov 19, 2024 · 05b435c · 05b435c
1 parent 8519982
commit 05b435c
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 4 deletions.
diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_resetsession.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_resetsession.cpp
@@ -89,7 +89,7 @@ bool TIndexTabletActor::PrepareTx_ResetSession(
         args.SessionSeqNo,
         args.Request.GetSessionState().size());
 
-    TIndexTabletDatabase db(tx.DB);
+    TIndexTabletDatabaseProxy db(tx.DB, args.NodeUpdates);
 
     bool ready = true;
     auto commitId = GetCurrentCommitId();
@@ -119,7 +119,7 @@ void TIndexTabletActor::ExecuteTx_ResetSession(
     TTransactionContext& tx,
     TTxIndexTablet::TResetSession& args)
 {
-    TIndexTabletDatabase db(tx.DB);
+    TIndexTabletDatabaseProxy db(tx.DB, args.NodeUpdates);
 
     auto* session = FindSession(args.SessionId);
     if (!session) {

diff --git a/cloud/filestore/libs/storage/tablet/tablet_tx.h b/cloud/filestore/libs/storage/tablet/tablet_tx.h
@@ -478,7 +478,7 @@ struct TTxIndexTablet
     // ResetSession
     //
 
-    struct TResetSession
+    struct TResetSession : TIndexStateNodeUpdates
     {
         /* const */ TRequestInfoPtr RequestInfo;
         const TString SessionId;
@@ -500,6 +500,7 @@ struct TTxIndexTablet
 
         void Clear()
         {
+            TIndexStateNodeUpdates::Clear();
             Nodes.clear();
         }
     };

diff --git a/cloud/filestore/libs/storage/tablet/tablet_ut_cache.cpp b/cloud/filestore/libs/storage/tablet/tablet_ut_cache.cpp
@@ -313,6 +313,60 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_NodesCache)
         UNIT_ASSERT_VALUES_EQUAL(5, statsAfter.RWCount - statsBefore.RWCount);
     }
 
+    Y_UNIT_TEST(ShouldUpdateCacheUponResetSession)
+    {
+        NProto::TStorageConfig storageConfig;
+        storageConfig.SetInMemoryIndexCacheEnabled(true);
+        storageConfig.SetInMemoryIndexCacheNodesCapacity(2);
+        storageConfig.SetInMemoryIndexCacheNodeRefsCapacity(2);
+        TTestEnv env({}, storageConfig);
+        env.CreateSubDomain("nfs");
+
+        ui32 nodeIdx = env.CreateNode("nfs");
+        ui64 tabletId = env.BootIndexTablet(nodeIdx);
+
+        TIndexTabletClient tablet(env.GetRuntime(), nodeIdx, tabletId);
+        tablet.InitSession("client", "session");
+
+        auto statsBefore = GetTxStats(env, tablet);
+
+        auto id = tablet.CreateNode(TCreateNodeArgs::File(RootNodeId, "test"))
+                      ->Record.GetNode()
+                      .GetId();
+        auto handle = tablet.CreateHandle(id, TCreateHandleArgs::RDWR);
+
+        // Cache hit
+        tablet.GetNodeAttr(id, "")->Record.GetNode();
+
+        tablet.UnlinkNode(RootNodeId, "test", false);
+        // Should work as there is an existing handle
+        tablet.GetNodeAttr(id, "");
+
+        // Upon reset session the node should be removed from the cache as well
+        // as the localDB
+        tablet.ResetSession("");
+
+        tablet.InitSession("client", "session2");
+
+        UNIT_ASSERT_VALUES_EQUAL(
+            E_FS_NOENT,
+            tablet.AssertGetNodeAttrFailed(id, "")->GetError().GetCode());
+
+        auto statsAfter = GetTxStats(env, tablet);
+        // First two GetNodeAttr calls should have been performed with the cache
+        UNIT_ASSERT_VALUES_EQUAL(
+            2,
+            statsAfter.ROCacheHitCount - statsBefore.ROCacheHitCount);
+        // Last GetNodeAttr call should have been a cache miss as it is not
+        // supposed to be present in the cache
+        UNIT_ASSERT_VALUES_EQUAL(
+            1,
+            statsAfter.ROCacheMissCount - statsBefore.ROCacheMissCount);
+        // CreateNode, CreateHandle, UnlinkNode, DestroySession and InitSession
+        // are RW txs
+        UNIT_ASSERT_VALUES_EQUAL(5, statsAfter.RWCount - statsBefore.RWCount);
+    }
+
     Y_UNIT_TEST(ShouldUpdateCacheUponSetNodeAttr)
     {
         NProto::TStorageConfig storageConfig;

diff --git a/cloud/filestore/libs/storage/tablet/tablet_ut_cache_stress.cpp b/cloud/filestore/libs/storage/tablet/tablet_ut_cache_stress.cpp
@@ -30,6 +30,7 @@ class TRequestGenerator
         CREATE_HANDLE,
         DESTROY_HANDLE,
         REBOOT_TABLET,
+        RESET_SESSION,
 
         OPERATION_COUNT
     };
@@ -106,8 +107,11 @@ class TRequestGenerator
                 case REBOOT_TABLET:
                     response = DoRebootTablet();
                     break;
-                default:
+                case RESET_SESSION:
+                    response = DoResetSession();
                     break;
+                default:
+                    ythrow yexception() << "must be unreachable";
             }
 
             if (!response.empty()) {
@@ -317,6 +321,11 @@ class TRequestGenerator
         return "Tablet rebooted";
     }
 
+    TString DoResetSession()
+    {
+        return Tablet->ResetSession("")->Record.DebugString();
+    }
+
     ////////////////////////////////////////////////////////////////////////////////
 
     // All timestamps after this timestamp are pruned, so we can safely