Skip to content

channels_sv2: make the past-jobs cap configurable - #2307

Open
gimballock wants to merge 3 commits into
stratum-mining:mainfrom
marafoundation:feat/configurable-max-past-jobs
Open

channels_sv2: make the past-jobs cap configurable#2307
gimballock wants to merge 3 commits into
stratum-mining:mainfrom
marafoundation:feat/configurable-max-past-jobs

Conversation

@gimballock

@gimballock gimballock commented Aug 18, 2026

Copy link
Copy Markdown

MAX_PAST_JOBS is really a retention window — cap ÷ template rate — and the template rate is a deployment property the library can't see. The same constant buys 50s of late-share tolerance at a 1s template cadence and over 16 minutes at 20s, so I don't think one number can be right for everyone.

Adds max_past_jobs: Option<usize> to the server and client ExtendedChannel/StandardChannel constructors. None and Some(0) both select MAX_PAST_JOBS, so nothing changes for existing callers.

Treating Some(0) as "no opinion" rather than as a literal zero keeps a zero cap unreachable: it would evict the job that just retired, so the most common late share would come back InvalidJobId with no startup error to warn you.

Group channels take no parameter — client GroupChannel has no past_jobs, and server group channels use replace_active_job.

A second commit will set the default once we've measured it; the analysis is coming in a comment.

Follow-up to #2290.

companion stratum-mining/sv2-apps#735

`MAX_PAST_JOBS` is a retention window -- `cap / template rate` -- and the
template rate is a deployment property this crate cannot observe. The same
constant buys 50s of late-share tolerance at a 1s template cadence and over 16
minutes at 20s, so no single value fits every deployment.

Add `max_past_jobs: Option<NonZeroUsize>` to the server and client
`ExtendedChannel` and `StandardChannel` constructors, resolved as
`max_past_jobs.map(NonZeroUsize::get).unwrap_or(MAX_PAST_JOBS)`. Passing `None`
preserves current behavior, so the constant stays the documented default rather
than being replaced.

`NonZeroUsize` rather than `usize`: a zero cap evicts the job that just retired,
so the most common late share is rejected as `InvalidJobId`, with no startup
error to warn the operator. Validating at runtime would mean adding `Result` to
the client constructors, which return `Self` today.

Group channels take no parameter -- client `GroupChannel` keeps no past jobs,
and server group channels replace the active job rather than retiring it, so the
cap is inert there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gimballock pushed a commit to marafoundation/sv2-apps that referenced this pull request Aug 19, 2026
DO NOT MERGE. Temporary commit so CI can build against the companion
stratum branch before it lands on stratum-mining/stratum.

Repoints the stratum-core git dependency from
stratum-mining/stratum#main to
marafoundation/stratum#feat/configurable-max-past-jobs (which carries the
max_past_jobs constructor parameter these changes depend on) in
stratum-apps and bitcoin-core-sv2, and regenerates the committed
lockfiles so the --locked CI builds resolve the fork.

Revert this commit once stratum-mining/stratum#2307 is merged and the
stratum-core pin is bumped to the merged rev.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Test the client `StandardChannel` override. That channel keeps its own
  `past_jobs`/`past_job_order` and eviction path rather than delegating to
  `JobStore`, so it was the one place `max_past_jobs` could regress unnoticed:
  the override was covered for the client extended channel and for the server via
  `JobStore`, but not here.

- Factor the resolver. `max_past_jobs.map(NonZeroUsize::get).unwrap_or(...)` was
  copy-pasted into four constructors; each side now routes through a
  `resolve_max_past_jobs` helper colocated with its own default, so changing a
  default touches one line rather than four.

- `debug_assert!(max_past_jobs > 0)` in `JobStore::new`. The nonzero guarantee
  lived only in the callers; this documents and enforces it internally without
  changing the signature.

- Rename the new tests off "honour" — the crate uses American spellings
  throughout.

- Make the client `MAX_PAST_JOBS` doc links explicit (`super::MAX_PAST_JOBS`) so
  they no longer depend on an import that the resolver made otherwise unused.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread sv2/channels-sv2/src/server/jobs/job_store.rs
Comment thread sv2/channels-sv2/src/client/standard.rs Outdated
@gimballock

Copy link
Copy Markdown
Author

Now that the cap (MAX_PAST_JOBS) is configurable, here's what it should default to — and why our data doesn't yet license moving it off 50.

Two losses, opposite fixes. An uncredited share is either an eviction (invalid-job-id, within a tip) or a cross-tip loss (stale-share, at a prev-hash rotation). If it's eviction, past_jobs must stay deep and the memory is unavoidable. If it's cross-tip loss, past_jobs can be tiny and the stale window sized on its own. Everything below serves one question: which one are we seeing?

Why our data can't decide

The costs are the ones already in #2290. On the memory side, at fixed connection count, cap 300 → 50 cut per-connection memory 36.5% (1.429 → 0.907 MiB, ~4.5 kB per retained job per channel). Both arms carried the same connection count within noise, so the cap moves density, not capacity — it buys headroom on the box rather than raising the connection ceiling. One caveat on that figure: per-connection memory isn't comparable across differing hardware, connection counts or trial lengths, and we published a 45.5% version of it that was wrong for exactly that reason. 36.5% is the corrected same-conditions result.

On the yield side, that same change cost 2.7 points on a channel-level metric we track — uncredited channels: those that opened successfully but never had a single share credited, 3.16% → 5.86%. Those channels stayed connected for the whole trial, so they were paying full per-connection memory while producing nothing creditable — a yield loss, not a connection loss. Both figures come from a gauge we've seen undercount under scrape pressure, so the 2.7-point delta is sturdier than either absolute.

Theory says the eviction path should be nearly empty. The required depth — how many past jobs the cap has to retain — is 1 + ceil(latency ÷ template period), which is 1 at the shipped min_interval = 5, and a code-level argument lands on the same 1–2 independently. But we can't attribute those 2.7 points of uncredited channels either way. Every simulated miner submitted one share seconds after connecting and then stopped, so no share was ever old enough to test the required depth — our invalid-job-id ≈ 0 is vacuous, not reassuring. That metric and stale-share rose together going 300 → 50, but under a one-shot workload that's correlation, not cause.

The experiment that decides

That uncredited-channel metric can't be reused: it only means anything because each channel submitted exactly once, and once miners resubmit after a rejection every channel eventually lands an accepted share, so the count collapses toward zero however much work is being lost. So this experiment needs to measure an actual stream of shares — rejected ÷ submitted, split by error code — under continuous hashing at fixed connection count. Those two counters are the discriminator, and previous runs simply never put load on them.

arm caps purpose
rare-tip 1, 2, 4, 50 eviction alone — locates the knee, the cap below which invalid-job-id starts climbing
rotating-tip 1, 2 adds the cross-tip path

Six runs. Without rotations, mark_past_jobs_as_stale never fires, so the rare-tip arm has no cross-tip path by construction and its losses are eviction only. The rotating arm has both. That gives two readings of the same question: the error-code split says which mechanism, and the rotating-minus-rare rejection rate at equal cap says how much is cross-tip loss — eviction appears in both arms and cancels.

That subtraction assumes eviction is equal across arms, which is close but not exact: each rotation drains past into stale, so the ring briefly can't evict while it refills, and the rotating arm suppresses eviction slightly. The bias runs against attributing loss to the past/stale coupling, so a signal that survives it is the stronger result.

The ladder is deliberately low. Eviction can only fire when a share arrives more than cap × template_period after retirement — 50 seconds of latency at cap 50 and a 1 s cadence, which is unreachable — so a ladder starting at 8 would report invalid-job-id ≈ 0 at every rung and prove nothing. Caps 1 and 2 are where the model says eviction must start biting, and they're the caps worth shipping; 50 stays as the tie-back to existing data. Low caps fill in seconds, so these are the cheap arms. Rotation rate gets reported alongside every stale-share figure, since rotating faster than mainnet inflates cross-tip loss.

Predictions, registered before the run:

if the loss is… invalid-job-id stale-share rotation sensitivity implication
eviction rises as cap falls, knee near 1 + ceil(L/T) flat low past_jobs stays deep; memory unavoidable
cross-tip loss flat rises as cap falls high past_jobs → ~2; size the stale window separately
neither flat flat the cap isn't the cause; the default can drop to the required depth

The third row is a real outcome, named in advance so it isn't treated as a failed run: rejection rates flat across the whole ladder would mean the earlier 2.7 points came from the one-shot workload rather than from the cap, and that nothing on the yield side argues against a small default.

Recommendation: keep the default at 50 until this runs. It's the safer side of a trade-off whose downside is unquantified below it. We wouldn't argue against a smaller cap on the eviction path; we can't vouch for it on the cross-tip path.

When it does run, the knee is a floor rather than a default. It's measured against one latency distribution and one cadence, so a shipped default wants headroom above it — and since the knee moves with both, the parameter this PR adds probably matters more than whichever number we end up recommending.

Contributions:

  • Measured job retention at ~4.5 kB per retained job per channel, the largest per-connection memory lever we've found.
  • Derived the required depth as 1 + ceil(latency ÷ template period), showing depth is set by latency rather than share rate or connection count.
  • Implicated the past/stale coupling rather than eviction, by splitting rejections per error code — a lead the experiment above is built to confirm or kill.
  • Specified a six-run experiment that separates the two loss paths, with predictions registered in advance.

Replaces `Option<NonZeroUsize>` with `Option<usize>` and drops the
`resolve_max_past_jobs` helpers, resolving inline in each constructor instead, as
requested in review.

`None` and `Some(0)` now both mean "no opinion" and select `MAX_PAST_JOBS`. That
keeps a zero cap unreachable — it would evict the job that just retired and reject
the most common late share as `InvalidJobId` — without the `NonZeroUsize` ceremony
at every call site, and it reads as the usual "0 means unset" config convention.

Tests: `Some(0)` is asserted equivalent to `None` on the server extended channel
and on both client channels. The server test arrives via a small `retained_past_jobs`
helper, which also closes a gap from the last round — the override was previously
only covered on the server side at the `JobStore` level, never through a channel
constructor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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