[Filestore] issue-5432: add basic async open support (vfs_fuse) by budevg · Pull Request #6588 · ydb-platform/nbs

budevg · 2026-07-23T10:01:48Z

#5432

github-actions · 2026-07-23T10:03:38Z

Note

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

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🔴 filestore (build failed, log)

github-actions · 2026-07-23T10:03:38Z

Note

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

Note

All workloads for linux-x86_64-release-ubsan have completed.

Tip

Planned checks for linux-x86_64-release-ubsan.

🔴 all (build failed, log)

github-actions · 2026-07-23T10:03:38Z

Note

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

Note

All workloads for linux-x86_64-release-tsan have completed.

Tip

Planned checks for linux-x86_64-release-tsan.

🔴 all (build failed, log)

github-actions · 2026-07-23T10:03:40Z

Note

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

Note

All workloads for linux-x86_64-release-asan have completed.

Tip

Planned checks for linux-x86_64-release-asan.

🔴 all (build failed, log)

github-actions · 2026-07-23T10:03:41Z

Note

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

Note

All workloads for linux-x86_64-release-msan have completed.

Tip

Planned checks for linux-x86_64-release-msan.

🔴 all (build failed, log)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ceaf58309

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-23T10:13:58Z

        };
    }

+    rpc ConfirmCreateHandle(TConfirmCreateHandleRequest) returns (TConfirmCreateHandleResponse) {


Add storage handling for the new confirmation RPC

This exposes ConfirmCreateHandle over gRPC, but the storage/Kikimr plumbing is not wired for the new method: cloud/filestore/libs/storage/api/service.h does not add a TEvService event id or include it in FILESTORE_SERVICE_REQUESTS, and the tablet/service actors dispatch only those request lists. In deployments using the real storage service rather than the null/local test handlers, async opens will call Session->ConfirmCreateHandle but there is no storage route/handler for the confirmation, so the queued handle confirmations cannot reach the tablet.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-23T10:13:58Z

    optional double FakeTxPageFaultsProbability = 527;
+
+    // Async processing of read-only create handle requests.
+    optional bool AsyncCreateHandleEnabled = 528;


Publish AsyncCreateHandleEnabled in session features

Adding this storage config knob is not enough to enable the vhost path: vfs_fuse/loop.cpp reads only TFileStoreFeatures::AsyncCreateHandleEnabled from CreateSession, but storage/core/config.*, tablet_actor_createsession.cpp, and service_local/fs_session.cpp do not expose an accessor or call features->SetAsyncCreateHandleEnabled(...). When this option is set in storage/local config, sessions still advertise false, so Open() never sets AllowAsyncCreateHandle outside tests that inject features directly.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-23T10:13:58Z

    xxx(ReadData,                           __VA_ARGS__)                       \
    xxx(WriteData,                          __VA_ARGS__)                       \
    xxx(AllocateData,                       __VA_ARGS__)                       \
+    xxx(ConfirmCreateHandle,                __VA_ARGS__)                       \


Preserve existing EFileStoreRequest values

Placing ConfirmCreateHandle here inserts it before Fsync/FsyncDir, endpoint requests, and the DescribeData/AddData blob requests, changing their numeric EFileStoreRequest values. Profile log records store only static_cast<ui32>(requestType), and the analytics/replay code casts that number back to EFileStoreRequest, so logs captured before this change for those later request types will be decoded or replayed as the wrong operation; append the new value after the existing range or assign stable explicit values instead.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-23T10:13:58Z


+    // If set, the handle was returned before durable persistence completed and
+    // the client must confirm it.
+    bool HandleCreatedAsync = 7;


Set HandleCreatedAsync in the real create path

The VFS queues a confirmation only when response.GetHandleCreatedAsync() is true, but this commit never wires any production CreateHandle path to set the new bit (repo search finds SetHandleCreatedAsync only in fs_ut.cpp). In the real tablet path an async read-only create can complete before durability via TabletUnsafeAsyncReadOnlyCreateHandleEnabled, yet the response still has this field false, so even with the client feature enabled the open is treated as persisted and no durable confirmation entry is written.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-23T10:13:58Z

+
+    // If set, the client allows CreateHandle to return before the handle is
+    // durably persisted.
+    bool AllowAsyncCreateHandle = 14;


Honor AllowAsyncCreateHandle before async completion

This new request bit is the only signal that the client can persist and later confirm an uncommitted handle, but the real tablet async branch still checks only TabletUnsafeAsyncReadOnlyCreateHandleEnabled and read-only flags (tablet_actor_createhandle.cpp:532) and does not read request.GetAllowAsyncCreateHandle() anywhere outside tests. With that storage option enabled, clients that did not set the bit, including vhost sessions where the async-create feature/queue is disabled, can still get a pre-durable handle and will never enqueue a confirmation.

Useful? React with 👍 / 👎.

SvartMetal · 2026-07-24T09:21:53Z

 message TQueueEntry
 {
     oneof Request {
          TCreateHandleRequest CreateHandleRequest = 1;


I think we can override this because we don't store TCreateHandleRequests anywhere yet

SvartMetal · 2026-07-24T09:41:48Z

+    with_lock (HandleOpsQueueLock) {
+        HandleOpsQueue->PopFront();
+    }
+    RetryDelayedRelease();


RetryDelayedRelease looks out of place in CompleteAsyncCreateHandle

SvartMetal · 2026-07-24T09:43:30Z

                });
+    } else if (entry.HasQueuedCreateHandleRequest()) {
+        const auto& requestInfo = entry.GetQueuedCreateHandleRequest();
+        auto request = CreateConfirmRequest(


CreateConfirmCreateHandleRequest

SvartMetal · 2026-07-24T09:46:19Z

+    ScheduleProcessHandleOpsQueue();
+}
+
+void TFileSystem::RetryDelayedRelease()


ProcessDelayedRelease?

SvartMetal · 2026-07-24T09:47:46Z

+    RetryDelayedRelease();
+    ScheduleProcessHandleOpsQueue();


It looks like these two calls are always used together

Maybe we should incorporate ScheduleProcessHandleOpsQueue into RetryDelayedRelease?

SvartMetal · 2026-07-24T10:11:17Z

    IMPLEMENT_DEFAULT_METHOD(AccessNode, NProto)
    IMPLEMENT_DEFAULT_METHOD(ReadLink, NProto)
    IMPLEMENT_DEFAULT_METHOD(RemoveNodeXAttr, NProto)
+    IMPLEMENT_DEFAULT_METHOD(ConfirmCreateHandle, NProto)


Looks like it can break existing profile logs

SvartMetal · 2026-07-24T10:22:39Z

+        // A release can be appended to DelayedReleaseQueue after a queue
+        // completion has already checked it. Retry here as well to avoid
+        // leaving that release stranded until another entry completes.
+        g.Release();
+        RetryDelayedRelease();


If we are fixing existing behavior, it would be better to do it in a separate PR

SvartMetal · 2026-07-24T10:29:44Z

+                STORAGE_ERROR(
+                    "ConfirmCreateHandle request failed: "
+                    << "filesystem " << self->Config->GetFileSystemId()
+                    << " error: " << FormatError(error));


It looks like such errors should never occur, so let's report a critical event

SvartMetal · 2026-07-24T10:30:52Z

+                        return;
+                    }
+
+                    ReportHandleOpsQueueProcessError(


I don't think this should be attributed to the handle ops queue. It looks like a separate critical event

SvartMetal · 2026-07-24T10:34:29Z

+
+        auto callContext = MakeIntrusive<TCallContext>(
+            Config->GetFileSystemId(),
+            CreateRequestId());


Should we use requestInfo.GetRequestId() here as well?

qkrorlqr · 2026-07-24T10:03:06Z

-        TEST_REQUEST_TYPE(57, WriteBlob);
-        TEST_REQUEST_TYPE(58, ConfirmAddData);
-        TEST_REQUEST_TYPE(59, CancelAddData);
+        TEST_REQUEST_TYPE(46, ConfirmCreateHandle);


seems that now all old logs will become incompatible with new logs (because of the change of event types of many requests which are already present in old logs) - doesn't look good

qkrorlqr · 2026-07-24T10:28:40Z

                SessionState,
                this);

+            FileSystem->Init();


why was this moved from Init()?

qkrorlqr · 2026-07-24T10:35:40Z

+                    << "filesystem " << self->Config->GetFileSystemId()
+                    << " error: " << FormatError(error));
+
+                if (GetErrorKind(error) == EErrorKind::ErrorRetriable) {


retriable errors are supposed to be handled at another layer - in DurableClient

qkrorlqr · 2026-07-24T10:37:19Z

+                        << " error: " << FormatError(destroyError));
+
+                    if (GetErrorKind(destroyError) ==
+                            EErrorKind::ErrorRetriable)


same comment about retriable errors

[Filestore] issue-5432: add basic async open support (vfs_fuse)

8ceaf58

budevg requested review from SvartMetal and qkrorlqr July 23, 2026 10:05

budevg mentioned this pull request Jul 23, 2026

[Filestore] issue-5432: add basic async open support #6580

Closed

chatgpt-codex-connector Bot reviewed Jul 23, 2026

View reviewed changes

SvartMetal requested changes Jul 24, 2026

View reviewed changes

qkrorlqr reviewed Jul 24, 2026

View reviewed changes

SvartMetal requested a review from WilyTiger July 24, 2026 10:39

Uh oh!

Conversation

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

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 23, 2026

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

budevg commented Jul 23, 2026 •

edited

Loading

github-actions Bot commented Jul 23, 2026 •

edited

Loading

github-actions Bot commented Jul 23, 2026 •

edited

Loading

github-actions Bot commented Jul 23, 2026 •

edited

Loading

github-actions Bot commented Jul 23, 2026 •

edited

Loading

github-actions Bot commented Jul 23, 2026 •

edited

Loading