MaxSessionHistoryId shouldn't be a global variable - session history …

…ids for different tablets are completely unrelated. This commit fixes a race as well. (#1677)
ydb-platform · Jul 26, 2024 · bb9ffa1 · bb9ffa1
1 parent b20fa49
commit bb9ffa1
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 27 deletions.
diff --git a/cloud/filestore/libs/storage/tablet/session.h b/cloud/filestore/libs/storage/tablet/session.h
@@ -30,7 +30,8 @@ struct TSessionHandle
     {}
 };
 
-using TSessionHandleList = TIntrusiveListWithAutoDelete<TSessionHandle, TDelete>;
+using TSessionHandleList =
+    TIntrusiveListWithAutoDelete<TSessionHandle, TDelete>;
 using TSessionHandleMap = THashMap<ui64, TSessionHandle*>;
 
 using TNodeRefsByHandle = THashMap<ui64, ui64>;
@@ -101,7 +102,7 @@ struct TSession
     TDupCacheEntryMap DupCache;
 
 public:
-    TSession(const NProto::TSession& proto)
+    explicit TSession(const NProto::TSession& proto)
         : NProto::TSession(proto)
         , SubSessions(GetMaxSeqNo(), GetMaxRwSeqNo())
     {}
@@ -122,7 +123,10 @@ struct TSession
         SetMaxRwSeqNo(SubSessions.GetMaxSeenRwSeqNo());
     }
 
-    NActors::TActorId UpdateSubSession(ui64 seqNo, bool readOnly, const NActors::TActorId& owner)
+    NActors::TActorId UpdateSubSession(
+        ui64 seqNo,
+        bool readOnly,
+        const NActors::TActorId& owner)
     {
         auto result = SubSessions.UpdateSubSession(seqNo, readOnly, owner);
         UpdateSeqNo();
@@ -230,7 +234,6 @@ struct TSession
     }
 };
 
-
 ////////////////////////////////////////////////////////////////////////////////
 
 struct TSessionHistoryEntry
@@ -243,7 +246,7 @@ struct TSessionHistoryEntry
         DELETE = 2
     };
 
-    TSessionHistoryEntry(const NProto::TSessionHistoryEntry& proto)
+    explicit TSessionHistoryEntry(const NProto::TSessionHistoryEntry& proto)
         : NProto::TSessionHistoryEntry(proto)
     {}
 
@@ -253,9 +256,13 @@ struct TSessionHistoryEntry
      * current system time. Type of an entry is set from `type`. EntryId (key)
      * is set as a first unused integer.
      */
-    TSessionHistoryEntry(const NProto::TSession& proto, EUpdateType type)
+
+    TSessionHistoryEntry(
+        const NProto::TSession& proto,
+        ui64 entryId,
+        EUpdateType type)
     {
-        SetEntryId(GetMaxEntryId());
+        SetEntryId(entryId);
         SetClientId(proto.GetClientId());
         SetSessionId(proto.GetSessionId());
         SetOriginFqdn(proto.GetOriginFqdn());
@@ -267,21 +274,6 @@ struct TSessionHistoryEntry
     {
         return ToString(static_cast<EUpdateType>(GetType()));
     }
-
-private:
-    // Incrementing counter for producing unique `EntryId`s
-    inline static ui64 UnusedSessionId = 1;
-
-    static ui64 GetMaxEntryId()
-    {
-        return UnusedSessionId++;
-    }
-
-public:
-    static void UpdateMaxEntryId(ui64 value)
-    {
-        UnusedSessionId = Max(UnusedSessionId, value + 1);
-    }
 };
 
 ////////////////////////////////////////////////////////////////////////////////

diff --git a/cloud/filestore/libs/storage/tablet/tablet_state_impl.h b/cloud/filestore/libs/storage/tablet/tablet_state_impl.h
@@ -41,6 +41,7 @@ struct TIndexTabletState::TImpl
     TSessionHandleMap HandleById;
     TSessionLockMap LockById;
     TSessionLockMultiMap LocksByHandle;
+    ui64 MaxSessionHistoryEntryId = 1;
 
     TWriteRequestList WriteBatch;
 

diff --git a/cloud/filestore/libs/storage/tablet/tablet_state_sessions.cpp b/cloud/filestore/libs/storage/tablet/tablet_state_sessions.cpp
@@ -114,8 +114,9 @@ void TIndexTabletState::LoadSessions(
     }
 
     for (const auto& entry: sessionsHistory) {
-        Impl->SessionHistoryList.push_back(entry);
-        TSessionHistoryEntry::UpdateMaxEntryId(entry.GetEntryId());
+        Impl->SessionHistoryList.emplace_back(entry);
+        Impl->MaxSessionHistoryEntryId =
+            Max(Impl->MaxSessionHistoryEntryId, entry.GetEntryId());
     }
 }
 
@@ -148,7 +149,10 @@ TSession* TIndexTabletState::CreateSession(
     db.WriteSession(proto);
     AddSessionHistoryEntry(
         db,
-        TSessionHistoryEntry(proto, TSessionHistoryEntry::CREATE),
+        TSessionHistoryEntry(
+            proto,
+            ++Impl->MaxSessionHistoryEntryId,
+            TSessionHistoryEntry::CREATE),
         SessionHistoryEntryCount);
     IncrementUsedSessionsCount(db);
 
@@ -307,7 +311,10 @@ void TIndexTabletState::ResetSession(
         db.WriteSession(*session);
         AddSessionHistoryEntry(
             db,
-            TSessionHistoryEntry(*session, TSessionHistoryEntry::RESET),
+            TSessionHistoryEntry(
+                *session,
+                ++Impl->MaxSessionHistoryEntryId,
+                TSessionHistoryEntry::RESET),
             SessionHistoryEntryCount);
     }
 }
@@ -325,7 +332,10 @@ void TIndexTabletState::RemoveSession(
     db.DeleteSession(sessionId);
     AddSessionHistoryEntry(
         db,
-        TSessionHistoryEntry(*session, TSessionHistoryEntry::DELETE),
+        TSessionHistoryEntry(
+            *session,
+            ++Impl->MaxSessionHistoryEntryId,
+            TSessionHistoryEntry::DELETE),
         SessionHistoryEntryCount);
 
     DecrementUsedSessionsCount(db);