fix(queue): raise River JobTimeout for large-document ingest#50
Conversation
River's client JobTimeout defaults to 1 minute, which cancels the ingest job mid-flight on large documents: summarize + HyDE + ToC issue one LLM call per section, and a 100+ section document needs far longer than 60s (it was failing with repeated 'context deadline exceeded' and retrying). Add a configurable RiverConfig.JobTimeout with a generous 30-minute default and pass it to the River client, so full-document summarization runs to completion.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a configurable ChangesRiver job timeout configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes Related PRs: None identified. Suggested labels: enhancement, queue Suggested reviewers: None identified. 🐰 A timer ticks where jobs once ran unbound, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/queue/river.go (2)
26-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc comment doesn't mention
-1disables the timeout.The comment says "Zero applies a generous default" but doesn't note that River also treats negative values (specifically
-1) as "no timeout" — which, given the<= 0check inNewRiver(Line 118), is currently indistinguishable from zero anyway. Worth clarifying once/if the check above is fixed to preserve that distinction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/queue/river.go` around lines 26 - 32, Update the JobTimeout doc comment to explicitly document the negative-value behavior alongside the zero default. The comment currently only says zero uses a generous default, but NewRiver also treats JobTimeout <= 0 as the default path, so clarify whether -1 is intended to mean “no timeout” and align the comment with the actual handling in NewRiver and JobTimeout. Keep the wording consistent with the existing timeout behavior and note the distinction if the <= 0 check is changed later.
26-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a unit test for the default-timeout branch.
NewRiver's zero/negative-to-30-minute defaulting logic (and the pass-through when a positive value is set) isn't covered by a dedicated test —TestRiverIntegrationinpkg/queue/river_test.gonever setsJobTimeoutand doesn't assert on it. A small unit test assertingcfg.JobTimeoutends up as 30 minutes when unset, and is preserved when explicitly set, would guard this behavior against regression.Also applies to: 118-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/queue/river.go` around lines 26 - 32, Add a focused unit test for NewRiver in pkg/queue/river_test.go that covers the JobTimeout defaulting behavior. Verify that when the config’s JobTimeout is zero or negative, NewRiver sets the effective timeout to 30 minutes, and that a positive JobTimeout is passed through unchanged. Use the NewRiver constructor and the JobTimeout field on the River config to keep the test aligned with the existing defaulting logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/queue/river.go`:
- Around line 118-123: In queue initialization, the JobTimeout fallback
currently treats River’s explicit no-timeout sentinel the same as an unset
value; update the logic in river.go so only cfg.JobTimeout == 0 gets defaulted
to 30 minutes. Keep the existing default assignment behavior for unset timeouts,
but preserve an explicit -1 by leaving it unchanged in the code path around
cfg.JobTimeout.
---
Nitpick comments:
In `@pkg/queue/river.go`:
- Around line 26-32: Update the JobTimeout doc comment to explicitly document
the negative-value behavior alongside the zero default. The comment currently
only says zero uses a generous default, but NewRiver also treats JobTimeout <= 0
as the default path, so clarify whether -1 is intended to mean “no timeout” and
align the comment with the actual handling in NewRiver and JobTimeout. Keep the
wording consistent with the existing timeout behavior and note the distinction
if the <= 0 check is changed later.
- Around line 26-32: Add a focused unit test for NewRiver in
pkg/queue/river_test.go that covers the JobTimeout defaulting behavior. Verify
that when the config’s JobTimeout is zero or negative, NewRiver sets the
effective timeout to 30 minutes, and that a positive JobTimeout is passed
through unchanged. Use the NewRiver constructor and the JobTimeout field on the
River config to keep the test aligned with the existing defaulting logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if cfg.JobTimeout <= 0 { | ||
| // Large documents (100+ sections) need well over River's 1-minute | ||
| // default to finish every per-section LLM summary. 30 minutes is a | ||
| // safe ceiling that still guards against a genuinely wedged job. | ||
| cfg.JobTimeout = 30 * time.Minute | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## river.go excerpt\n'
sed -n '1,180p' pkg/queue/river.go
printf '\n## river dependency/version\n'
rg -n 'river|github.com/riverqueue/river' go.mod go.sum . -g '!**/vendor/**' || trueRepository: hallelx2/vectorless-engine
Length of output: 23663
🌐 Web query:
River JobTimeout -1 disable timeout documented
💡 Result:
In the River job queue library for Go, setting JobTimeout to -1 is the documented way to disable job timeouts [1][2]. When JobTimeout is set to -1: - The job's context will not be cancelled by the River client due to a timeout [2]. - The job will continue to run indefinitely unless the Client itself is shut down (e.g., via a graceful shutdown) [2]. This configuration is available at both the client-wide level (in the river.Config struct) and the individual worker level (by returning -1 in the worker's Timeout method) [1]. While this functionality is supported, it is generally recommended to use timeouts to ensure jobs remain cancellable and to avoid issues with stuck jobs [1][3].
Citations:
- 1: https://riverqueue.com/docs/reliable-workers
- 2: https://github.com/riverqueue/river/blob/master/client.go
- 3: https://riverqueue.com/docs/maintenance-services
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## search for JobTimeout usage/docs in repo\n'
rg -n 'JobTimeout|job timeout|disable timeout|timeout sentinel|-1' . -g '!**/vendor/**' || trueRepository: hallelx2/vectorless-engine
Length of output: 16123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## River config struct and validation\n'
sed -n '320,430p' pkg/config/config.go
printf '\n---\n'
sed -n '1238,1260p' pkg/config/config.go
printf '\n## docs and examples mentioning river timeout\n'
rg -n 'job[_-]?timeout|timeout' config.example.yaml config.server.example.yaml README.md docs pkg/config -g '!**/vendor/**' || trueRepository: hallelx2/vectorless-engine
Length of output: 6938
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## river package config usage around JobTimeout\n'
sed -n '220,250p' pkg/queue/river.go
printf '\n## any tests for RiverConfig or timeout behavior\n'
rg -n 'JobTimeout|NewRiver\\(|RiverConfig' pkg/queue pkg/config -g '*_test.go' || trueRepository: hallelx2/vectorless-engine
Length of output: 1254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## search River docs for -1 sentinel and default semantics from source files in repo comments\n'
sed -n '1,120p' docs/ENGINE.md | sed -n '295,320p' || trueRepository: hallelx2/vectorless-engine
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## NewRiver call sites\n'
rg -n 'NewRiver\\(' . -g '!**/vendor/**' || true
printf '\n## River config field definitions and defaults\n'
sed -n '680,750p' pkg/config/config.goRepository: hallelx2/vectorless-engine
Length of output: 2145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## NewRiver call sites and River config translation\n'
rg -n 'NewRiver\\(|RiverConfig\\{' cmd internal pkg -g '!**/vendor/**' || true
printf '\n## relevant config River block\n'
sed -n '330,380p' pkg/config/config.goRepository: hallelx2/vectorless-engine
Length of output: 1881
Preserve River's -1 timeout sentinel. cfg.JobTimeout <= 0 also rewrites an explicit no-timeout setting to 30 minutes; switch this to == 0 so only the unset zero value gets the default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/queue/river.go` around lines 118 - 123, In queue initialization, the
JobTimeout fallback currently treats River’s explicit no-timeout sentinel the
same as an unset value; update the logic in river.go so only cfg.JobTimeout == 0
gets defaulted to 30 minutes. Keep the existing default assignment behavior for
unset timeouts, but preserve an explicit -1 by leaving it unchanged in the code
path around cfg.JobTimeout.
Resolves the govulncheck finding: a webp alpha-channel panic in golang.org/x/image reachable via the PDF parser's image support.
1 participant
River's client
JobTimeoutdefaults to 1 minute, cancelling the ingest job mid-flight on large documents. Summarize + HyDE + ToC each issue one LLM call per section, so a 100+ section document needs far longer than 60s — it was failing with repeatedcontext deadline exceededand retrying until discarded.Adds a configurable
RiverConfig.JobTimeout(generous 30-minute default) passed to the River client, so full-document summarization runs to completion.Verified live: a 91-section document now summarizes in ~98s and reaches
ready(previously died at the 60s mark).Summary by CodeRabbit