lock state files for session persistent tables #4870

akilvein · 2025-12-17T18:47:55Z

Add option for TPersistentTable to lock the state file. Avoid state corruption if case of several vhosts accidentally start with the same parameters on the same machine.

github-actions · 2025-12-17T18:48:05Z

Hi! Thank you for contributing!
The tests on this PR will run after a maintainer adds an ok-to-test label to this PR manually. Thank you for your patience!

github-actions · 2025-12-31T11:36:26Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 39947be.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3123	3123	0	0	0	0	0

github-actions · 2025-12-31T13:54:40Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 614e9b5.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3273	3273	0	0	0	0	0

budevg · 2026-01-25T15:35:51Z

cloud/filestore/libs/service_local/session.h

+    NProto::TError TryInit(bool restoreClientSession)
    {
        auto handlesPath = StatePath / "handles";
+        auto indexPath = StatePath / "nodes";


You are leaking implementation details from TLocalIndex into TSession. TSession shouldn't know that there is a "nodes" file which is used by the TIndex

budevg · 2026-01-25T15:58:40Z

cloud/filestore/libs/service_local/session.h

        auto handlesPath = StatePath / "handles";
+        auto indexPath = StatePath / "nodes";
+
+        if (!TryLockFile(handlesPath, HandlesFileLock) ||


Locking handles here is premature because the file may be deleted later:

if (isNewSession) { ... handlesPath.DeleteIfExists();

budevg · 2026-01-25T16:05:38Z

cloud/filestore/libs/service_local/fs_session.cpp

        return makeResponse(*it->second);
    }

    auto clientSessionStatePath = StatePath / ("client_" + clientId);


Actually simplest and safest fix is to create a dedicated lock file in clientSessionStatePath (e.g. session.lock or state.lock) and never delete it. Then lock that file to guard the whole session state.

budevg · 2026-01-25T16:19:01Z

cloud/filestore/libs/service_local/service_ut.cpp

    {
-        TTestBootstrap bootstrap;
-        bootstrap.CreateFileStore("fs", "cloud", "folder", 100500, 500100);
+        TTempDirectoryPtr bootstrapCwd;


Is there actually a new test that tests that it's forbidden to run two instances of same TSession ?

ShouldFailToLockSessionStateTwice

budevg · 2026-01-25T16:37:29Z

cloud/filestore/libs/service_local/service_ut.cpp


    Y_UNIT_TEST(ShouldRecoverSessionHandles)
    {
-        TTestBootstrap bootstrap("fs", "client");


To avoid extra refactoring in UTs, add CloseStore to TTestBootstrap and call it explicitly when needed.

StopFileStore

budevg · 2026-01-25T16:39:14Z

cloud/filestore/libs/service_local/service_ut.cpp

+        TString expectedSessionId;

-        for (auto& file: files) {
-            auto handle =


Need to test this in devlab as well. Run filestore-vhost-local create fs, start endpoint attach vm. Then run another filestore-vhost-local process and see what happens

… files

github-actions · 2026-01-26T00:42:05Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 0fb8d64.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3318	3317	0	1	0	0	0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 0fb8d64.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
19	17	0	2	0	0	0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 0fb8d64.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
19	19	0	0	0	0	0

github-actions · 2026-01-26T19:17:37Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit c7cc5f8.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3321	3319	0	2	0	0	0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit c7cc5f8.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
19	16	0	2	0	1	0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit c7cc5f8.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
19	19	0	0	0	0	0

budevg · 2026-01-28T09:32:21Z

cloud/filestore/libs/service_local/session.h

-    void Init(bool restoreClientSession)
+    virtual ~TSession()
+    {
+        UnlockFile(SessionFileLock);


Do we need destructor here ? The TFileLock will close the file when it's destroyed and this will unlock it automatically

budevg · 2026-01-28T09:33:35Z

cloud/filestore/libs/service_local/session.h

+        UnlockFile(SessionFileLock);
+    }
+
+    NProto::TError TryInit(bool restoreClientSession)


we can keep calling the function Init() and just add return value

budevg · 2026-01-28T10:22:23Z

cloud/filestore/libs/service_local/fs_session.cpp

    }

    auto clientSessionStatePath = StatePath / ("client_" + clientId);
    clientSessionStatePath.MkDir();


Actually let's just move the whole session state lock logic here. Create TFileLock to guard the session state, lock it and move it to TSession. When the TSession object destroyed the file lock will be automatically released. Also let's do unit tests in session_ut.cpp

akilvein force-pushed the users/akilvein/persistent-table-files branch 3 times, most recently from eedeac6 to 276f827 Compare December 18, 2025 15:57

debnatkh added ok-to-test Label to approve test launch for external members filestore Add this label to run only cloud/filestore build and tests on PR labels Dec 31, 2025

github-actions bot removed the ok-to-test Label to approve test launch for external members label Dec 31, 2025

akilvein force-pushed the users/akilvein/persistent-table-files branch from 39947be to 614e9b5 Compare December 31, 2025 12:59

budevg self-requested a review January 22, 2026 14:36

budevg requested changes Jan 25, 2026

View reviewed changes

Andrei Kilvein and others added 7 commits January 25, 2026 22:17

lock state files for session persistent tables

e7f0361

draft error handling

0bb4492

improve test + remove redundant try/catch

a499cd3

fix tests + LocalIndex::Init

638a3fc

Add legal error handling for local service session init

47f714c

simplify locking logic

94bc346

Lock dedicated "session.lock" file instead of persistend tables state…

0fb8d64

… files

akilvein force-pushed the users/akilvein/persistent-table-files branch from 614e9b5 to 0fb8d64 Compare January 25, 2026 23:29

fix tests

c7cc5f8

budevg requested changes Jan 28, 2026

View reviewed changes

lock state files for session persistent tables #4870

Are you sure you want to change the base?

lock state files for session persistent tables #4870

Uh oh!

Conversation

akilvein commented Dec 17, 2025

Uh oh!

github-actions bot commented Dec 17, 2025

Uh oh!

github-actions bot commented Dec 31, 2025

Uh oh!

github-actions bot commented Dec 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jan 26, 2026 •

edited

Loading

github-actions bot commented Jan 26, 2026 •

edited

Loading