Skip to content

x/sharedfile: promote public, add Ref and Retire#2239

Open
pjbgf wants to merge 4 commits into
mainfrom
packhandle
Open

x/sharedfile: promote public, add Ref and Retire#2239
pjbgf wants to merge 4 commits into
mainfrom
packhandle

Conversation

@pjbgf

@pjbgf pjbgf commented Jul 3, 2026

Copy link
Copy Markdown
Member

Makes the reference-counted file-handle primitives usable by external
consumers and extends them so a caller can safely share a memory-mapped
file across concurrent readers and tear it down without racing in-flight
reads.

SharedFile already ref-counts a lazily-opened file with grace-period /
pool-driven close, but it was internal and offered no durable
reference or graceful teardown. A consumer that memory-maps large
read-only files (e.g. packfiles) and shares them across concurrent
operations needs to (a) hold a mapping open for an operation's lifetime,
(b) bound open fds/mmaps across many files, and (c) retire a file that
has been superseded without munmapping it out from under a reader that is
still using it. These changes provide exactly those, without altering the
existing SharedFile/fdpool contracts.

Four commits:

  1. x/sharedfile: promote from internal. Moves the package to
    x/ so it can be imported outside the module. Pure relocation —
    internal/packhandle and plumbing/format/idxfile just switch import
    paths; no behavior change.

  2. x/sharedfile: add durable Ref. A single-owner, idempotent
    handle over Acquire/Release. While a Ref (or cursor) is live the
    underlying fd/mapping cannot be torn down by the grace timer,
    ReleaseNow, Retire, or pool eviction. Lets a caller capture a file
    once and read through it for the capture's whole lifetime instead of
    re-acquiring per read.

  3. x/sharedfile: add graceful Retire. A teardown that seals the
    handle against new Acquire/reopen and closes it — immediately if no
    references are held, otherwise deferred to the last Release so
    in-flight readers ride to completion. It uses a sealed state
    distinct from closed, so IsClosed() stays accurate for readers
    mid-flight. Distinct from Close (hard, closes under refs) and
    ReleaseNow (permits reopen on next Acquire). This is the
    forget-on-publish primitive: retire a superseded file, let current
    readers finish, and the fd/mapping drops when the last one releases.

  4. x/fdpool: over-commit instead of force-evicting live pins. When
    every eviction candidate is pinned, the pool now retains over capacity
    (recorded in Stats.OverCommits) rather than force-evicting a pinned
    member — which frees no fd (a pinned member's ReleaseNow only
    latches) and would only churn accounting and defeat reuse. Non-pinned
    members are evicted as before; the LRU self-corrects as pins release.

Copilot AI review requested due to automatic review settings July 3, 2026 08:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Promotes the shared, ref-counted file-handle primitive into the public x/sharedfile package, adds new lifecycle primitives (Ref, Retire) to support safe concurrent mmap-style usage, and updates x/fdpool eviction behavior to avoid force-evicting pinned members.

Changes:

  • Promote SharedFile into x/sharedfile and update internal import sites accordingly.
  • Add SharedFile.Ref() / Ref for durable, single-owner pinning across an operation lifetime.
  • Add SharedFile.Retire() for graceful “seal + deferred close” teardown and adjust fdpool to over-commit when all candidates are pinned.

Reviewed changes

Copilot reviewed 12 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
x/sharedfile/sharedfile.go Adds sealed state and implements graceful Retire() semantics; updates Acquire/Release behavior accordingly.
x/sharedfile/sharedfile_test.go Adds comprehensive tests for SharedFile core behaviors (grace period, concurrency, ReleaseNow).
x/sharedfile/sharedfile_pool_test.go Adds tests covering pool wiring (LRU touch, eviction, concurrent read/eviction stress).
x/sharedfile/sharedfile_bench_test.go Adds benchmarks to track Acquire/Release overhead (with/without pool, last-drop path).
x/sharedfile/retire_test.go Adds tests validating Retire() sealing + deferred-close behavior.
x/sharedfile/ref.go Introduces the durable Ref type and SharedFile.Ref() constructor.
x/sharedfile/ref_test.go Adds tests ensuring Ref pins through grace period and is idempotent on close.
x/sharedfile/doc.go Adds package documentation for x/sharedfile.
x/fdpool/pool.go Changes eviction policy to over-commit instead of force-evicting when all members are pinned; adds OverCommits stat.
x/fdpool/pool_test.go Updates/extends tests to cover over-commit behavior when all members are pinned.
plumbing/format/idxfile/lazy_index.go Switches import from internal/sharedfile to x/sharedfile.
internal/packhandle/testutil_test.go Switches import from internal/sharedfile to x/sharedfile.
internal/packhandle/source.go Switches import from internal/sharedfile to x/sharedfile.
internal/packhandle/packhandle.go Switches import from internal/sharedfile to x/sharedfile.
internal/packhandle/cursor_reader.go Switches import from internal/sharedfile to x/sharedfile.
internal/packhandle/cursor_reader_test.go Switches import from internal/sharedfile to x/sharedfile.

Comment thread x/fdpool/pool_test.go Outdated
pjbgf added 3 commits July 3, 2026 09:46
Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
When every eviction candidate is pinned, retain all members over
capacity and increment Stats.OverCommits rather than calling
ReleaseNow on a pinned Member. A pinned Member's ReleaseNow only
latches (never closes while in use), so force-eviction drops honest
accounting and defeats cross-request fd reuse without freeing anything.
The LRU self-corrects as pins release on subsequent Touch calls.

Replace TestPool_AllPinned_FallsBackToLRU (old force-evict behaviour)
with TestPool_AllPinned_OverCommits and add TestPoolOverCommitsWhenAllPinned
to cover the self-correction path (unpinned after over-commit is evicted
on the next Touch).

Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants