fix(jobcontroller): give each job reconnect its own deadline

[kuangren777]

Problem

onConnectionUp creates a single 5-second context and reuses it for every ReconnectJob call in a serial loop:

ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second)
defer cancelFn()
...
for _, job := range jobsToReconnect {
    err = ReconnectJob(ctx, job.OID, nil)

The deadline covers the whole batch rather than each job. On a high-latency connection only the first few jobs finish before it elapses; every remaining job then fails at the same instant with context deadline exceeded and its block stays stalled. Those jobs are not retried — the only recovery is switching the block's connection to local and back, which tears down the conncontroller and starts a fresh job.

Reproduction

macOS client to a Linux host over a Tailscale DERP-relayed link, ~85ms RTT (relay-only, no direct path). 15 durable jobs on the connection. Laptop sleeps, sshd reaps the session, laptop wakes:

11:13:43.960  [conn:...] connection became connected, reconnecting jobs
11:13:43.425  RemoteReconnectToJobManagerCommand: successfully reconnected to job manager
11:13:45.270  RemoteReconnectToJobManagerCommand: successfully reconnected to job manager
11:13:46.372  RemoteReconnectToJobManagerCommand: successfully reconnected to job manager
11:13:49.014  [job:...] error reconnecting: failed to get job: context deadline exceeded   <- x12, same millisecond
11:13:49.014  [conn:...] finished reconnecting jobs: 3/15 successful

All 12 failures land in the same millisecond, which is the shared deadline expiring rather than 12 independent timeouts. Measured cost of one successful reconnect on this link: 0.55s and 1.31s. 5s divided by ~1.5s is about 3 jobs, matching 3/15.

On a LAN a reconnect costs roughly 30-50ms, so the same 5s budget covers 100+ jobs and the bug is invisible. It only shows up over WAN/relayed links, where it makes durable blocks look permanently hung after every sleep/wake.

Fix

Give each reconnect its own deadline and run them with bounded concurrency. Recovery then depends on the slowest single job rather than on job count multiplied by RTT.

Notes

• sync and panichandler were already imported; no new dependencies.
• ReconnectJob already dedups per job id via singleflight.Group, so concurrent calls for distinct job ids are safe.
• ReconnectJobTimeout (15s) and ReconnectJobConcurrency (8) are exported consts. Happy to tune these or make them configurable.
• go vet ./pkg/jobcontroller/... and go build -race ./pkg/jobcontroller/ are clean.
• No test added: pkg/jobcontroller currently has no test files, and onConnectionUp reaches directly into the wstore global DB and the package-level ReconnectJob. If you want coverage here I am happy to follow up with a small refactor that makes the reconnect function injectable, so the batching semantics can be tested without a live DB or connection.

Verification

Built and run on macOS arm64 against the same setup. Forced a disconnect by killing the sshd session, with 9 durable jobs attached:

13:02:21.575  [conn:...] connection became connected, reconnecting jobs
13:02:21.577  [conn:...] found 9 jobs to reconnect
13:02:21.784  [conn:...] finished reconnecting jobs: 9/9 successful

209ms for 9 jobs, averaging 23ms each, which is below the cost of a single reconnect round trip on this link -- the reconnects overlapped as intended. Before the change the same connection reported 3/15 successful after 5.6s.

Re-tested under injected latency to reproduce the original failure condition. Added tc qdisc add dev tailscale0 root netem delay 100ms on the remote host, bringing RTT to 107ms (measured rtt min/avg/max/mdev = 106.945/107.466/108.317/0.512 ms), with 13 durable jobs attached:

13:06:02.648  [conn:...] connection became connected, reconnecting jobs
13:06:02.654  [conn:...] found 13 jobs to reconnect
13:06:03.988  [conn:...] finished reconnecting jobs: 13/13 successful

13/13 in 1.34s. 13 jobs over a concurrency of 8 is two waves, ~0.67s each, which matches the per-job cost at this RTT. The pre-change serial loop would need ~8.7s for the same set and would have abandoned the tail at the shared 5s deadline.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5aba30ba-0147-4a5d-a7b6-a895c64cb9fb

📥 Commits

Reviewing files that changed from the base of the PR and between 544914a and 3cadccb.

📒 Files selected for processing (1)

• pkg/jobcontroller/jobcontroller.go

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

Walkthrough

The job controller adds a 15-second per-job reconnect timeout and a concurrency limit of eight. onConnectionUp now reconnects jobs concurrently with independent timeout contexts. It logs failures, handles panics, protects the success counter, and waits for all reconnect attempts to finish.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 3cadc

The PR gives each job its own reconnect deadline and limits concurrent reconnects, improving recovery for high-latency connections; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: assigning each job reconnect its own deadline.
Description check	✅ Passed	The description directly explains the shared-deadline problem, the bounded-concurrency fix, and verification results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}