Skip to content

fix(fs): wait for mount readability before returning background mounts - #69

Merged
qiffang merged 3 commits into
mainfrom
fix/fs-mount-ready-timeout
Oct 2, 2026
Merged

qiffang merged 3 commits into
mainfrom
fix/fs-mount-ready-timeout

Conversation

@mornyx

@mornyx mornyx commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

ti fs mount-file-system and ti fs-vault mount-vault returned as soon as the ti-drive9 companion mount process exited successfully. On a real backend the freshly mounted path is not yet usable at that point, so first reads inside the warmup window can fail with EAGAIN (Resource temporarily unavailable) and succeed only after a few seconds.

Reproduced on macOS against a live us-west-2 backend with --driver fuse (background mount + concurrent first reads on never-cached content):

  • t=+0.70s after mount returned: cat <mounted-file> → Resource temporarily unavailable (1 of 60 concurrent reads)
  • t=+0.64s: lookup of an uncached path returned EAGAIN instead of its real result; the same lookup returned the correct ENOENT from t=+2.7s on
  • sequential blocking reads never failed, and content read through any earlier mount never failed (companion-side caches persist across mounts), which is why CI did not catch this

Root cause

--ready-timeout was declared and parsed into MountFileSystemOptions.ReadyTimeout / VaultMountOptions.ReadyTimeout, but the live Drive9 companion paths (drive9MountFileSystem, drive9MountVault) never read it — a dead flag. AGENTS.md and README both promise that background mounts "wait until the mount is ready"; nothing enforced that after the mount runtime moved to the companion. Drive9's own background mount gate only probes the mount root and does not warm the data plane (its FUSE layer deliberately maps canceled/timed-out requests and HTTP 5xx to EAGAIN — mem9-ai/drive9#1006).

Scope

This is an independent, cross-driver CLI readiness/timeout hardening change: when these commands exit successfully, an active kernel mount exists at the requested path and its root is readable, bounded by --ready-timeout. A successful root readdir proves exactly that. It does not guarantee cold-file first stat/open/read or error-free concurrent scans during companion warmup — those remain Drive9-side semantics (mem9-ai/drive9#1012 tracks the FUSE first-initial_sync directory race, #1006 the errno mapping) and remain retryable by design.

Change

  • New internal/fs/mountready.go: after the companion mount succeeds and the mount locator is written, poll the mount path until it is an active mount that can be listed. Each probe is stat + active-mount evidence + one readdir entry (EOF accepted), capped by min(10s, remaining --ready-timeout) and the command context, so a short --ready-timeout never waits a full probe bound and Ctrl-C during a blocked probe returns promptly.
  • Active-mount evidence is platform-appropriate: st_dev comparison against the parent directory on macOS (mountready_darwin.go) and /proc/self/mountinfo with octal-escape decoding on Linux (mountready_linux.go); unsupported platforms fall back to readability only because ti mounts are unsupported there.
  • On timeout the command fails with fs.mount_ready_timeout; on Ctrl-C with fs.mount_ready_canceled. In both cases the background mount is left running, the mount locator is preserved, and the error carries the exact unmount command for that mount path.
  • Wired into both drive9MountFileSystem and drive9MountVault.
  • The fake ti-drive9 companions used by unit and e2e tests now create a readable mount path. Black-box e2e runs pass mount evidence via the hidden TI_TEST_FAKE_MOUNT_READY=1 control, gated by TI_ALLOW_TEST_ENDPOINTS=1 like the existing TI_TEST_* overrides.
  • README documents the readiness contract and the failure semantics.

Testing

  • Unit (internal/fs/mountready_test.go):
    • probe semantics: plain and empty directories are rejected without mount evidence; missing/non-directory paths error; readability passes with evidence.
    • discriminating timeout/cancellation tests for blocked probes: a probe blocked past a 200ms --ready-timeout is abandoned at the budget (not the 10s probe bound), and cancellation 100ms into a blocked probe returns fs.mount_ready_canceled promptly.
    • regression TestDrive9MountRequiresActiveMountEvidence: companion exits 0 leaving only a plain directory → fs.mount_ready_timeout, locator preserved (fails against the previous implementation, which reported mounted).
    • TestDefaultMountPointActiveAcceptsRealWebDAVMount (darwin): mounts a real mount_webdav volume backed by an x/net/webdav server and proves the predicate is false before the mount and true only after the kernel mount exists and is readdirable.
    • timeout/locator-preservation coverage for both fs and fs-vault mounts, plus cancellation.
  • make test — all packages pass. make e2e — pass with the updated fake companion and test gate.
  • Live verification against a real us-west-2 backend (fresh tenant, cold content, CDN companion), macOS + macFUSE:
    • FUSE, cold tenant (the original repro conditions): mount returned in 4.07s — ti held the return until the mount root was actually readable — and ls at +0.056s after return succeeded; a 60-way concurrent read storm plus rg walk plus a read hammer produced zero EAGAIN (pre-fix, the same scenario produced EAGAIN at +0.70s). One transient deep-directory EIO surfaced in the concurrent rg walk during companion warmup; that is Drive9-side behavior outside ti's control (see Scope).
    • WebDAV (default on macOS), cold session: mount returned in 9.07s after ti waited out the ~6.4s kernel session/readir warmup; ls at +0.056s succeeded; the same storm was fully clean. (An earlier 2s per-probe bound false-timed-out this run; fixed in d668e2e.)
    • Timeout path (live): with a mount that never became readable, the command failed after 30s with fs.mount_ready_timeout, the error carried the exact unmount command, the locator survived, that unmount command succeeded, and a follow-up --ignore-absent unmount reported absent.
    • Re-verified after the evidence rework (bf07eb0): the platform detection accepts real drive9 mounts on both drivers (WebDAV 6.71s and FUSE 3.07s waits, kernel mount table confirmed, readable at return, unmount clean).
    • Post-run cleanup verified: mount drain/unmount and tenant delete against the live backend.

Notes

  • The published English ti docs live in a submodule that is not initialized in this checkout; the documented behavior ("wait until the mount is ready") is unchanged and now actually enforced, so no page update is required.
  • Drive9-side warmup semantics are untouched: in-flight requests issued by other processes while the mount command is still waiting can still see transient EAGAIN/EIO (FUSE: align errno with Linux/ext4 and add syscall differential tests mem9-ai/drive9#1006, #1012). After this change, ti no longer reports success before an active, readable mount exists.

The --ready-timeout flag on ti fs mount-file-system and ti fs-vault
mount-vault was parsed but never consumed on the Drive9 companion
path: the commands returned as soon as the companion mount process
exited successfully, so first reads during the companion warmup window
could surface EAGAIN from the freshly mounted path.

After the companion mount succeeds and the mount locator is written,
poll the mount path until directory listing succeeds, bounded by
--ready-timeout (default 30s). On timeout or cancellation the mount is
left running, the locator is preserved, and the error carries the
exact unmount command. The fake companions used by unit and e2e tests
now create a readable mount path so they model a live background
mount.
@pingcap-cla-assistant

pingcap-cla-assistant Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot ti-chi-bot Bot added the size/XL label Oct 2, 2026
Live verification against a real backend caught a false timeout: a
WebDAV mount's first directory listing crosses the companion proxy and
the remote region and can legitimately exceed the previous 2s
per-probe bound, so every probe was abandoned as not-ready until the
overall --ready-timeout fired even though the mount was healthy. Raise
the per-probe bound to 10s; its purpose is only to stop a wedged mount
from blocking a probe forever.

@qiffang qiffang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REVISE on d668e2e0a7e9a85076ed2ff143e701328ff8777f.

The direction has real value. The live runs used the pre-#1012 companion (0f0f301) and show that a post-companion wait can prevent an early return, especially while a cold WebDAV kernel session is still warming up. Wiring the previously unused --ready-timeout, preserving the locator, and providing an unmount hint are useful CLI improvements.

Two correctness blockers remain before merge:

  1. --ready-timeout and cancellation do not bound an in-flight probe.

    probeMountPointOnce always permits a probe to block for 10 seconds. waitForMountReady checks its own deadline only after that probe returns, and checks ctx.Done() only while sleeping between probes. Therefore --ready-timeout=1s or 5s can still take about 10 seconds, and Ctrl-C during a blocked probe can also be delayed by up to 10 seconds. This contradicts the README claim that the wait is bounded by --ready-timeout.

    Please propagate the context/deadline into each probe and cap every probe by the remaining overall budget (and any earlier context deadline). Add discriminating tests for a blocked probe with a short timeout and for cancellation while the probe is in flight.

  2. The readiness predicate and tests do not prove that a mount exists.

    probeMountPointReady succeeds for any ordinary readable directory; TestProbeMountPointReady explicitly treats an empty temp directory as ready, and the fake companion only performs MkdirAll plus a marker write. A companion that exits 0 without mounting anything therefore passes the new tests and can still produce status: mounted.

    Please require observable active-mount evidence appropriate to each supported driver/platform, and add a regression where “companion succeeded but only a normal directory exists” remains not ready. The test must fail on the old behavior and pass only after a real mount becomes active/readable.

Scope also needs to stay precise: a successful root readdir proves only that the root was readable at that instant. It does not guarantee cold-file stat/open/read or later concurrent scans. Drive9 #1012 owns the known FUSE first-initial_sync directory race; this PR should be described and tested as an independent cross-driver CLI readiness/timeout hardening change, not as a complete fix for all post-mount cold-read EAGAIN cases.

@mashenjun

Copy link
Copy Markdown
Collaborator

@codex review

…vidence

Address review feedback on the mount readiness wait:

- Every probe is now capped by min(10s, remaining --ready-timeout) and
  observes the command context, so a short --ready-timeout no longer
  waits a full probe bound and Ctrl-C during a blocked probe returns
  promptly instead of after the probe budget.
- Readiness now requires active-mount evidence, not just a readable
  directory: st_dev comparison against the parent on macOS and
  /proc/self/mountinfo on Linux. Unsupported platforms fall back to
  readability only because ti mounts are unsupported there.
- New regression: a companion that exits 0 without mounting leaves a
  plain directory that must not satisfy readiness. A darwin test mounts
  a real mount_webdav volume and proves the predicate passes only after
  the kernel mount exists.
- Black-box e2e fake-companion runs enable mount evidence via the
  hidden TI_TEST_FAKE_MOUNT_READY control, gated by
  TI_ALLOW_TEST_ENDPOINTS like the other TI_TEST_* overrides.
@ti-chi-bot ti-chi-bot Bot removed the size/XL label Oct 2, 2026
@mornyx

mornyx commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

@qiffang Thanks for the careful review — both blockers are addressed in bf07eb0.

1. --ready-timeout and cancellation now bound every in-flight probe.

probeMountPointOnce takes the context and a per-attempt budget of min(10s, remaining --ready-timeout), and its select observes ctx.Done(). waitForMountReady recomputes the remaining budget before each attempt, so:

  • --ready-timeout=1s returns at ~1s even if the probe never answers (the probe goroutine is abandoned).
  • Ctrl-C during a blocked probe returns fs.mount_ready_canceled immediately instead of after the probe budget.

Discriminating tests added: TestWaitForMountReadyBoundsBlockedProbeByTimeout (blocked probe, 200ms timeout, asserts the wait does not take the old 10s probe bound) and TestWaitForMountReadyCancelsBlockedProbe (cancellation 100ms into a 5s-blocked probe, asserts prompt fs.mount_ready_canceled).

2. Readiness now requires real mount evidence.

The probe is stat + active-mount evidence + readdir:

  • macOS: st_dev of the mount path vs its parent — a mount root sits on a different filesystem, a plain subdirectory does not (mountready_darwin.go).
  • Linux: /proc/self/mountinfo with octal-escape decoding, the kernel's authoritative table (mountready_linux.go).
  • Other platforms: readability-only fallback, since ti mounts are unsupported there.

Regressions added:

  • TestDrive9MountRequiresActiveMountEvidence — companion exits 0 and only a plain directory exists; the command must fail with fs.mount_ready_timeout instead of reporting mounted. This fails against the previous implementation.
  • TestDefaultMountPointActiveAcceptsRealWebDAVMount — spins up an x/net/webdav server, mounts it with the real mount_webdav, and proves the predicate is false before the mount and true (and readdirable) only after the kernel mount exists. It also covers the case where the served tree contains the mountpoint, which deadlocks kernel WebDAV — the test keeps them separate.
  • TestDefaultMountPointActiveRejectsPlainDirectory and the reworked TestProbeMountPointReady subtests pin the plain-directory rejection on every platform.

e2e fake companion: the fake cannot create a real kernel mount, so black-box e2e runs pass evidence via the hidden TI_TEST_FAKE_MOUNT_READY=1 control, gated by TI_ALLOW_TEST_ENDPOINTS=1 like the existing TI_TEST_* overrides.

Scope: agreed — I've reframed the PR description as cross-driver CLI readiness/timeout hardening. A successful root readdir proves the mount exists and the root is readable at that instant; cold-file first reads and concurrent scans during companion warmup remain Drive9-side behavior (#1012 for the FUSE first-initial_sync directory race, #1006 for the errno mapping).

Verification after the change: unit + make test + make e2e green; live re-verified against a real us-west-2 backend on macOS — the evidence predicate accepts real drive9 mounts on both drivers (WebDAV: mount returned after 6.71s having waited out the cold kernel session, kernel mount table confirmed, ls at +0.07s; FUSE: 3.07s, same shape), and unmount still routes through the locator.

@qiffang qiffang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review round 1 — exact head bf07eb0f. Verdict: APPROVED.

Clean, well-scoped fix: a background mount now waits for actual readability before the command returns, instead of returning a not-yet-listable mount. Verified by code reading + build + local logic tests; the one real-mount test is CI-covered.

The implementation is correct and bounded

  • probeMountPointReady proves readiness with Stat → IsDir → mount-table evidence → Readdirnames(1) on the root — a background mount is only "ready" once it's an active mount the kernel can list.
  • probeMountPointOnce runs the probe in a goroutine and abandons it on budget/ctx expiry (a wedged probe can't block forever; the orphaned goroutine closes its handle when the syscall returns).
  • waitForMountReady poll loop is bounded by deadline = now + timeout: each probe budget = min(mountReadyProbeTimeout 10s, remaining); breaks when remaining ≤ 0 → fs.mount_ready_timeout apperr; ctx cancel/deadline → distinct fs.mount_ready_canceled; poll interval clamped to remaining. No unbounded wait.
  • Platform mounted evidence is correct: linux reads /proc/self/mountinfo (authoritative), matches raw/clean/eval-symlinks/abs candidates, and octal-unescapes mountinfo paths (\040→space); darwin compares st_dev of the path vs its parent (a mount root is on a different device), root-path special-cased. mountready_other.go covers unsupported platforms.
  • Wired into the real background-mount return paths — drive9_companion.go:1196 (fs-vault) and :1302 (fs), each with opts.ReadyTimeout and a helpful stop hint; the --ready-timeout flag (default 30s) is plumbed through commands.go.

Verification

  • go build ./... clean.
  • All mountready LOGIC tests pass locally: TestProbeMountPointReady (+subtests), TestDefaultMountPointActiveRejectsPlainDirectory, and the wait/budget/cancellation tests → ok.
  • TestDefaultMountPointActiveAcceptsRealWebDAVMount performs a real mount_webdav + probe. It t.Skips when the env can't mount; in my sandbox mount_webdav half-succeeded (mount created but not usable), so the test didn't skip and the probe correctly reported "budget exhausted" — an environment artifact, not a logic defect. The repo's test CI (proper mount env) is green, covering it.
  • MERGEABLE; test + license/cla checks green.

No remaining blocker. LGTM.

@qiffang qiffang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction to my earlier APPROVE (exact head bf07eb0f). Verdict: CHANGES_REQUESTED — test-only; the production code is APPROVE-able.

reviewer3's independent gatekeeper pass caught a factual error in my prior review, which I verified and am correcting here.

What I got wrong

My earlier review claimed the repo's test CI "covers" TestDefaultMountPointActiveAcceptsRealWebDAVMount. That is false: .github/workflows/ci.yml's test job is runs-on: ubuntu-latest (Linux only). The test file has no build tag so it compiles on Ubuntu, but mount_webdav does not exist there, so the test hits t.Skipf(...) and is skipped on CI — it is exercised by no CI job. I incorrectly dismissed the local hang as "a sandbox artifact that CI covers"; it isn't covered.

The real test defect (should-fix before merge)

On darwin (where mount_webdav exists) TestDefaultMountPointActiveAcceptsRealWebDAVMount is environment-fragile: when mount_webdav returns 0 but the mount's first readdir wedges, the test fails after the 10s probe budget and the context-free deferred umount then hangs; reviewer3 observed repeated runs leaking 5 webdavfs_agent processes + 5 WebDAV mounts. So make test on darwin can hang and leak system mounts. Please make it an explicit opt-in darwin integration test (or safe-skip on a mounted-but-unusable env) and give the cleanup a bounded/force umount fallback, so make test neither fails, hangs, nor leaks.

The production code is sound (no code blocker)

I and reviewer3 independently confirmed: bounded readiness probe + poll loop (per-probe budget = min(10s, remaining), ctx-honoring, goroutine-abandon for wedged probes — bounded, not a leak in steady state), Linux /proc/self/mountinfo matching (octal-unescape + candidate set), Darwin st_dev comparison, and both real return paths (drive9_companion.go:1196/:1302) with --ready-timeout plumbed. The non-real-mount logic tests all pass. Once the test above is fixed (opt-in/safe-skip + bounded cleanup), this is an APPROVE on the code.

@qiffang
qiffang merged commit 0c1c564 into main Oct 2, 2026
2 checks passed
@mornyx
mornyx deleted the fix/fs-mount-ready-timeout branch October 3, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants