Skip to content

IPFS pin task retains its full object list while parked, so retention is unbounded across repos (#174 follow-up) #296

Description

@beardthelion

Summary

PR #174 adds a global pin_semaphore that bounds how many post-push pin loops run concurrently. On the local IPFS replication path the per-push object list is materialized and moved into the detached task before that permit is acquired, so a task parked waiting for a permit still holds its full list. EncryptInflight caps parked tasks at one per repo, so nothing bounds the retained memory across distinct repos.

This is a follow-up to #174, not a defect on main: origin/main has no pin_semaphore, no EncryptInflight, and no pin_new_objects_gated.

The branch already states this gap in its own docstrings, deliberately, in four places (pin_new_objects_gated in api/repos.rs, the pin_semaphore and encrypt_inflight field docs in state.rs, and the max_concurrent_pin_tasks doc in config.rs). This issue exists to track that declared residual, not to report it as undisclosed. It is the sibling of #287, which tracks the same pool's hold time on the Pinata side.

Mechanism

At head 807bc93, all in crates/gitlawb-node/src/api/repos.rs unless noted:

  • The receive-pack tail materializes object_list at repos.rs:2154-2170.
  • It moves that list into the detached task: tokio::spawn(run_encrypt_pin_task(ctx, inflight_guard, object_list, ...)) at repos.rs:2213.
  • run_encrypt_pin_task (repos.rs:880) hands the snapshot to pin_and_encrypt_objects (repos.rs:888), which calls pin_new_objects_gated (repos.rs:1185) taking the list by value.
  • The permit is acquired at repos.rs:1160, after the parked future already owns the list.
  • EncryptInflight (crates/gitlawb-node/src/state.rs:286) keys in-flight tasks by repo id, one per repo. There is no global cap on the parked set.
  • The pool is max_concurrent_pin_tasks, default 8 (crates/gitlawb-node/src/config.rs:396, wired at main.rs:409).

The Pinata task on the same tail orders it the other way: it acquires at repos.rs:2305 and re-derives its list inside the permit via pinata_object_list_for_refs at repos.rs:2308.

Reachability

Pushing is authenticated (git_receive_pack, repos.rs:1640, takes Extension<AuthenticatedDid>). Two things widen that:

What limits it, stated plainly: accumulation requires the 8-slot pin pool to be saturated and slow, which needs a configured IPFS endpoint under load. With GITLAWB_IPFS_API empty (the default, config.rs:88) each permit is held only long enough to no-op, so lists do not pile up on a stock node. The realistic worst case is a node with IPFS configured and the endpoint degraded: pushes to N distinct repos retain N object-id lists (roughly 64 bytes per object id, so tens of MB for a very large push) for as long as the pool stays full. Authenticated but unprivileged, not anonymous.

What is and is not bounded today

Bounded: how many pin loops run concurrently (pin_semaphore, default 8); in-flight pin tasks per repo (one, via EncryptInflight); coalesced pending tip pairs per repo (MAX_PENDING_TIP_PAIRS = 1024, state.rs:300, over fixed-width 40-hex pairs, so about 80 KB); and the IPFS pin loop's own hold, via the batch budget added in #174.

Not bounded: the object-list memory held by tasks parked on pin_semaphore, summed across repos.

Also not bounded, and worth naming because it sits on the path usually cited as the good example: the Pinata task captures full ref names (ref_updates_clone, repos.rs:2251), parse_ref_updates (repos.rs:2755) applies no length check to the ref name, a pkt-line permits one near 65 KB, and that task is deliberately not coalesced (see the comment at repos.rs:2222-2231), so its parked count is one per push with no cap. The Pinata ordering avoids retaining the object list; it does not make that path free of retention.

Three fix shapes that were tried and do not work

  1. Capture the ref tuples instead of the list. Does not bound retention. parse_ref_updates (repos.rs:2755) checks that both SHAs are 40 characters and applies no bound to the ref name, and the request body is unlimited, so one captured tuple can carry a name near 65 KB.
  2. Cap that capture at MAX_PENDING_TIP_PAIRS (1024). Bounds count, not bytes. That constant is byte-safe only because its current contents are fixed-width 40-hex SHA pairs (PendingWork::Tips, state.rs:305). Applied to a capture containing ref names it is a count bound with nothing behind it.
  3. Derive the list after the park, mirroring the Pinata twin. Needs a fresh repo-identity re-read, because the disk path is keyed on the owner/name slug and a delete plus re-create between spawn and drain gives the row a new id (see the comment at repos.rs:1023). Abandoning on an id mismatch then drops a pin the coalesced drain would otherwise have kept, and there is no reconciliation sweep to recover it (Implement the reconciliation sweep the replication path already assumes as a durability backstop #218). That is the failure class Coalesced drain drops pending pin work permanently on a transient DB error #289 already documents on the drain path.

What a real fix has to bound

A byte bound on what a parked task retains, not a count bound, and not a bound that trades retention for a dropped pin. It needs to hold under all three of: a ref name near the pkt-line maximum, a push whose object list is large, and pushes spread across many distinct repos so the per-repo cap does not bind.

Acceptance should be a retention assertion (total retained bytes across parked tasks stays under a stated ceiling while the pool is saturated), not a task count, since a count assertion passes today while the bytes are unbounded. Whatever lands must not reintroduce the drop-on-mismatch path from shape 3.

Refs #174. Sibling of #287 (same pool, hold time rather than retention). Related to #217, which moves the same spawn for a different reason, and to #218, whose absence is what makes dropping work in shape 3 permanent.

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:nodegitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorsev:mediumDegraded but workaround existssubsystem:peersPeer announce, discovery, and registrysubsystem:replicationMirror, replica, and cross-node syncsubsystem:storageBlob/object store, Arweave, IPFS, archives

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions