issue-1751: [Filestore] WriteBackCache: Replace ReadWriteRangeLock with pinning mechanism, move remaining code to TUtils by e673 · Pull Request #5297 · ydb-platform/nbs

e673 · 2026-02-23T17:33:52Z

Part of #5064

Replace TReadWriteRangeLock with pinning mechanism. Instead of blocking at overlapping read and write operations, the reader now sets a pin that prevents cached data from being evicted from cache. This makes possible to:

avoid copying cached data parts under mutex;
avoid using temporary buffer for storing cached data.

Additionally, some helper functions were moved to TUtils.

github-actions · 2026-02-23T17:48:00Z

Note

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

🟢 linux-x86_64-relwithdebinfo target: cloud/disk_manager/ (test time: 224s): all tests PASSED for commit eab0cbf.

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

🟢 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 241s): all tests PASSED for commit eab0cbf.

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

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1538s): all tests PASSED for commit eab0cbf.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
5812	5811	0	0	0	1	0

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 4740s): all tests PASSED for commit eab0cbf.

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

…th pinning mechanism, move remaining code to TUtils

SvartMetal · 2026-02-26T15:55:46Z

cloud/filestore/libs/vfs_fuse/write_back_cache/node_state.h


-    // Prevent from concurrent read and write requests with overlapping ranges
-    TReadWriteRangeLock RangeLock;
+    // Prevents flushed requests from being evicted from Cache


Let’s emphasize why this logic is needed

Added more info about why this is needed

SvartMetal · 2026-02-26T16:00:03Z

cloud/filestore/libs/vfs_fuse/write_back_cache/write_back_cache.cpp

-        IFileStorePtr session,
-        ISchedulerPtr scheduler,
-        ITimerPtr timer,
-        IWriteBackCacheStatsPtr stats,
-        TLog log,
-        const TString& fileSystemId,
-        const TString& clientId,
-        const TString& filePath,
-        ui64 capacityBytes,
-        TDuration automaticFlushPeriod,
-        TDuration flushRetryPeriod,
-        ui32 maxWriteRequestSize,
-        ui32 maxWriteRequestsCount,
-        ui32 maxSumWriteRequestsSize,
-        bool zeroCopyWriteEnabled)


Unnecessary diff

Reverted (I will replace it with DTO in the future)

SvartMetal · 2026-02-26T16:00:59Z

cloud/filestore/libs/vfs_fuse/write_back_cache/write_back_cache_state.cpp

+    auto& nodeState = Nodes.GetOrCreateNodeState(nodeId);
+
+    auto it = nodeState.CachedDataPins.find(pinId);
+    Y_ENSURE(


Let's use Y_ABORT_UNLESS

github-actions · 2026-02-26T17:10:47Z

Note

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

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 4739s): all tests PASSED for commit 9647acd.

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

github-actions · 2026-02-26T21:39:55Z

Note

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

🟢 linux-x86_64-release-asan target: cloud/filestore/ (test time: 3968s): all tests PASSED for commit e334109.

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

github-actions · 2026-02-26T21:56:17Z

Note

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

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 4820s): all tests PASSED for commit e334109.

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

github-actions · 2026-02-26T22:06:39Z

Note

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

🟢 linux-x86_64-release-msan target: cloud/filestore/ (test time: 3788s): all tests PASSED for commit e334109.

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

github-actions · 2026-02-26T22:10:39Z

Note

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

🟢 linux-x86_64-release-tsan target: cloud/filestore/ (test time: 4075s): all tests PASSED for commit e334109.

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

github-actions · 2026-02-26T22:16:31Z

Note

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

🟢 linux-x86_64-release-ubsan target: cloud/filestore/ (test time: 3772s): all tests PASSED for commit e334109.

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

qkrorlqr · 2026-02-27T12:30:54Z

cloud/filestore/libs/vfs_fuse/write_back_cache/write_back_cache_state.cpp

+
+    // Prevent unflushed requests from being evicted after flush
+    const ui64 sequenceId =
+        nodeState.Cache.GetMinPendingOrUnflushedSequenceId(0);


0 /* paramName */

Removed the default value and changed logic a bit.
Now all Get...SequenceId methods act the same.

qkrorlqr · 2026-02-27T12:34:58Z

cloud/filestore/libs/vfs_fuse/write_back_cache/write_back_cache_state_ut.cpp

        UNIT_ASSERT_VALUES_EQUAL("", b.VisitUnflushedCachedRequests(1));
    }

+    Y_UNIT_TEST(PinCachedData)


let's add a test with multiple pins per 1 sequence id

This scenario is already covered by the test
Added more comments inside

neihar · 2026-02-27T13:32:55Z

cloud/filestore/libs/vfs_fuse/write_back_cache/write_back_cache_state.cpp

+{
+    auto guard = Guard(QueuedOperations);
+
+    auto& nodeState = Nodes.GetOrCreateNodeState(nodeId);


GetNodeState? and check that exist?

cloud/filestore/libs/vfs_fuse/write_back_cache/utils.cpp

e673 changed the base branch from main to users/nasonov/wbc-refactor-9 February 23, 2026 17:35

e673 requested review from SvartMetal, debnatkh, neihar and qkrorlqr and removed request for debnatkh February 23, 2026 17:35

Base automatically changed from users/nasonov/wbc-refactor-9 to main February 26, 2026 15:39

issue-1751: [Filestore] WriteBackCache: Replace ReadWriteRangeLock wi…

9647acd

…th pinning mechanism, move remaining code to TUtils

e673 force-pushed the users/nasonov/wbc-refactor-10 branch from eab0cbf to 9647acd Compare February 26, 2026 15:42

SvartMetal reviewed Feb 26, 2026

View reviewed changes

Review

e334109

e673 added asan Launch builds with address sanitizer along with regular build tsan Launch builds with thread sanitizer along with regular build labels Feb 26, 2026

e673 added msan Launch builds with memory sanitizer along with regular build ubsan Launch builds with undefined behaviour sanitizer along with regular build labels Feb 26, 2026

e673 requested a review from SvartMetal February 27, 2026 10:56

SvartMetal previously approved these changes Feb 27, 2026

View reviewed changes

qkrorlqr reviewed Feb 27, 2026

View reviewed changes

neihar reviewed Feb 27, 2026

View reviewed changes

Review

5a8c6e0

e673 dismissed SvartMetal’s stale review via 5a8c6e0 February 27, 2026 14:43

Review: UnpinCachedData

e85ff49

e673 requested review from SvartMetal, neihar and qkrorlqr February 27, 2026 15:47

Conversation

e673 commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 23, 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!

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 Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

e673 commented Feb 23, 2026 •

edited

Loading

github-actions bot commented Feb 23, 2026 •

edited

Loading