issue-1751: [Filestore] Allow deletion from TFileRingBuffer from any position

[e673]

Notes

TFileRingBuffer supports entry removal only from head of the ring buffer but TWriteBackCache needs to be able to randomly allocate and free memory.

Previously, this was managed at TWriteBackCache side — it stored deleted entries but its actial deletion occured when they become front entries.

The problem was that information about deleted entries was not persisted — after nfs-vhost restart deleted entries could be resurrected and flushed again. This didn't break data consistency but handles associated with the resurrected requests could be destroyed and the cache could become stuck (infinite E_FS_BADHANDLE at flush).

In this PR, the logic is moved from TWriteBackCache to TFileRingBuffer and deletion flag is persisted.

Issue

#1751

[Copilot]

Pull request overview

This PR extends TFileRingBuffer to support freeing allocations from arbitrary positions (by persisting a per-entry “skip” flag), and wires that capability into the write-back cache persistent storage to avoid resurrecting deleted requests after restart.

Changes:

• Add TFileRingBuffer::Free() and persist a deletion/skip flag inside entry headers; update traversal/pop logic to ignore skipped entries.
• Refactor write-back cache persistent storage to delegate deletion to TFileRingBuffer (remove the in-memory DeletedEntries tracking) and make Free()/Commit() results enforced at call sites.
• Update and extend unit tests to cover v3→v4 format migration and random deletion persistence across restarts.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cloud/storage/core/libs/common/file_ring_buffer.h	Adds Free() API and documents skip-flag behavior.
cloud/storage/core/libs/common/file_ring_buffer.cpp	Implements skip-flagged entries, pointer→position map, v4 format bump/migration, and random deletion.
cloud/storage/core/libs/common/file_ring_buffer_ut.cpp	Adds helpers and tests for v3→v4 compatibility and random deletion persistence.
cloud/filestore/libs/vfs_fuse/write_back_cache/write_data_request_manager.cpp	Enforces Commit()/Free() success for persistent storage operations.
cloud/filestore/libs/vfs_fuse/write_back_cache/write_back_cache_ut.cpp	Adjusts stats test helper logic after persistent deletion behavior change.
cloud/filestore/libs/vfs_fuse/write_back_cache/test/test_persistent_storage.h	Updates Free() signature to return bool.
cloud/filestore/libs/vfs_fuse/write_back_cache/test/test_persistent_storage.cpp	Implements Free() returning false on invalid/double free.
cloud/filestore/libs/vfs_fuse/write_back_cache/persistent_storage.h	Marks Alloc/Commit/Free as [[nodiscard]] and changes Free() to return bool.
cloud/filestore/libs/vfs_fuse/write_back_cache/persistent_storage.cpp	Removes DeletedEntries and delegates deletion to TFileRingBuffer::Free().
cloud/filestore/libs/vfs_fuse/write_back_cache/persistent_storage_ut.cpp	Updates tests to expect Free() failure on double free rather than exceptions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Note

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

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

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED?
3642	3642	0	0	0	0	0

[github-actions]

Note

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

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

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED?
3642	3642	0	0	0	0	0

[github-actions]

Note

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

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

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED?
3642	3642	0	0	0	0	0

[github-actions]

Note

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

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

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED?
3687	3687	0	0	0	0	0

[robot-vibe-db]

AI Review Summary

Verdict: ❌ 1 critical issue(s) found

Critical issues

• Critical | Medium: EraseFreeEntriesFromFront() silently clears the entire buffer on corruption instead of preserving remaining data and setting the Corrupted flag — potential data loss — cloud/storage/core/libs/common/file_ring_buffer.cpp:503

Other findings

• Major | Medium: Migrate() crashes the process (Y_ABORT_UNLESS) on V3 files with corruption where bit 31 of DataSize is set, instead of graceful degradation — cloud/storage/core/libs/common/file_ring_buffer.cpp:435
• Minor | High: Stale documentation for Commit() — comment still says "Returns true/false" but signature was changed to void — cloud/filestore/libs/vfs_fuse/write_back_cache/persistent_storage.h:54
• Minor | Medium: ShouldSupportRandomDeletion test does not cover Free() with ring buffer data wrapping (ReadPos > WritePos) — cloud/storage/core/libs/common/file_ring_buffer_ut.cpp:977

---

This review was generated automatically. Critical issues require attention; other findings are advisory.
If this comment was useful, please give it a 👍 — it helps us improve the review bot.

[robot-vibe-db]

Severity: Minor
Confidence: High

Stale documentation: the comment still says "Returns true if the commit was successful. Returns false if Alloc was not called." but the method signature was changed from bool to void in this PR. The method now throws (Y_ENSURE) on failure instead of returning false.

Suggested fix: update the comment to reflect the new behavior:

/**
 * Commits previous memory allocation.
 *
 * Memory that was allocated but not committed will be lost at buffer
 * recreation.
 *
 * Throws if Alloc was not called before Commit.
 */
virtual void Commit() = 0;

[github-actions]

Note

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

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

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED?
3687	3687	0	0	0	0	0