Skip to content

Pin permit's worst-case hold is unbounded on the Pinata side (#174 follow-up) #287

Description

@beardthelion

Summary

PR #174 caps concurrent pin work with a global pin_semaphore that DEFERS rather than sheds. The IPFS loop is now bounded (a batch budget, a per-iteration gate, and the remainder passed as each request's timeout). The Pinata task holds the same permit and is not bounded, so the pool's worst case is unchanged in magnitude.

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

The hold

api/repos.rs:2266 clones state.pin_semaphore (the same pool the IPFS path takes at repos.rs:1146) and acquires at repos.rs:2274. The permit spans both the re-derivation and the pin loop.

Before the loop runs, pinata_object_list_for_refs can park on the scan pool three times sequentially, each a bare acquire_owned().await with no timeout and no shed arm (state.rs:852):

  1. replication_withheld_set -> acquire_scan_permit (api/repos.rs:81, "withheld walk")
  2. resolve_candidates_for_push -> acquire_scan_permit (git/push_delta.rs:323, "pin-candidate scan")
  3. on the full-scan arm, fail_closed_full_scan_objects (api/repos.rs:139, "fail-closed full scan")

Each stage's git children are bounded by git_service_timeout_secs, default 600s (config.rs:180). So the hold before the loop starts is three unbounded queue waits plus up to ~3 x 600s.

Then the loop itself (pinata.rs:79) has no deadline, no per-request override, and warns-and-continues (pinata.rs:121). Each HTTP add inherits the shared client's 10s (main.rs:1102), so the per-request side is fine; the loop total is O(N objects) with N chosen by the pusher.

Two gates that spend a permit to do nothing

Pinata, when unconfigured. do_pinata_replication = announce_at_root && !own_walk_failed (repos.rs:2250) does not check the JWT. pinata_jwt defaults empty (config.rs:65), so a stock node takes the permit, parks on the scan pool, runs the whole re-derivation, and only then hits the early return at pinata.rs:87.

This cannot be fixed by adding the condition to that binding. repos.rs:2273 is let (announce, pinned) = if do_pinata_replication { ... } else { (false, Vec::new()) };, and announce gates four downstream systems: gossip (repos.rs:2327), the GraphQL subscription broadcast (:2355), Arweave anchoring (:2372), and peer-notify (:2416). Folding the JWT in suppresses all four on every unconfigured node. No existing test would catch it: the three announce-asserting tests all route through the f2a_pinata fixture, which sets a JWT (repos.rs:8694). The fix has to split the pin gate from the announce gate.

IPFS, when unconfigured. pin_new_objects_gated gates on object_list.is_empty() but not ipfs_api.is_empty() (repos.rs:1131-1150), and ipfs_api defaults empty (config.rs:61). The comment directly above that gate (repos.rs:1138) states the principle it does not apply: "parking here would spend a global pin slot on no work. The pool DEFERS rather than sheds, so those calls stall pins for every other repo."

Note pin_new_objects_gated_defers_when_pin_pool_exhausted (repos.rs:5893) passes "" as ipfs_api specifically to make the pin a no-op while still exercising deferral, so it needs a non-empty endpoint to keep testing what it means to test. That is a deliberate test update, not a regression.

Two residuals the shipped docstring already names

  • The git read. store::read_object composes two bare std::process::Command::output() calls with no timeout and no process-group reaping (git/store.rs:276, :308), called from async with no spawn_blocking at ipfs_pin.rs:249, pinata.rs:103, and encrypted_pin.rs:155. Bounded twins exist (store.rs:435, :541) and are used only by the /ipfs path. This half is pre-existing and present on main. Adjacent to Read/fork handlers run blocking git subprocesses on the tokio runtime (no spawn_blocking, no timeout) #204, which is the same class scoped to fork and read handlers.
  • DB round-trips. is_pinned, record_pinned_cid, has_pinata_cid, record_pinata_cid are bounded only by the 5s pool acquire (config.rs:224). There is no statement_timeout anywhere in the crate; the only mention is a comment noting its absence (api/ipfs.rs:192), which the /ipfs path works around with explicit timeout wrappers the pin path lacks.

Acceptance

A hold-time assertion on the Pinata holder, not a concurrency count, matching the shape #282 uses. Splitting the pin gate from the announce gate needs a test that an unconfigured node still announces.

Refs #174. Companion to #282 (write-guard hold time). The truncation semantics the IPFS loop documents are #218's.

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:nodegitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorsev:highMajor break or real security/trust risk, no easy workaroundsubsystem:replicationMirror, replica, and cross-node sync

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions