Skip to content

Size active queried series worker pool from GOMAXPROCS#7671

Open
anxkhn wants to merge 1 commit into
cortexproject:masterfrom
anxkhn:fix/active-queried-series-gomaxprocs
Open

Size active queried series worker pool from GOMAXPROCS#7671
anxkhn wants to merge 1 commit into
cortexproject:masterfrom
anxkhn:fix/active-queried-series-gomaxprocs

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 5, 2026

Copy link
Copy Markdown
**What this PR does**:

The active queried series service (added in #7173) sizes its worker pool with
`runtime.NumCPU()`:

```go
numWorkers := max(min(runtime.NumCPU()/2, 4), 1)

runtime.NumCPU() returns the number of logical CPUs on the host and does not
account for the CPU cgroup quota. Cortex imports go.uber.org/automaxprocs
(cmd/cortex/main.go) precisely so that GOMAXPROCS reflects the container's
CPU limit, and elsewhere it reads container-aware parallelism via
runtime.GOMAXPROCS(0), for example the resource monitor's CPU limit
(pkg/cortex/modules.go) and the engine's decoding-concurrency default, which
is documented as GOMAXPROCS / 2 (pkg/engine/config.go).

This one call site is inconsistent with that convention. On a many-core host
with the ingester restricted to a small CPU quota via cgroups, it sizes the
pool off the host core count rather than the CPU budget (for example, on a
64-core host limited to 2 CPUs it computes min(64/2, 4) = 4 workers instead
of the min(GOMAXPROCS/2, 4) the rest of the code would use).

This PR switches the call site to runtime.GOMAXPROCS(0) so the pool follows
the container CPU quota, matching the rest of the codebase. The min(..., 4)
cap is unchanged, so the blast radius is small. A table-driven regression test
asserts the worker count follows GOMAXPROCS.

Which issue(s) this PR fixes:

No open issue; this is a small self-contained correctness fix. Happy to open a
tracking issue first if maintainers prefer.

Checklist

  • Tests updated
  • Documentation added (n/a: no config/flags or user-facing docs change)
  • CHANGELOG.md updated
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags (n/a: no flags)

This change was prepared with AI assistance (per the project's GENAI policy);
I reviewed every line, confirmed the behavior against the codebase's existing
automaxprocs/GOMAXPROCS(0) usage, and verified the regression test is red
against runtime.NumCPU() and green with the fix.


> Note (do NOT paste): the CHANGELOG entry currently carries the `#XXXX`
> placeholder. Per repo memory, replace it with the real PR number immediately
> after the PR is opened (amend the CHANGELOG line and push).

---

## Diff summary (what ships)

3 files, +33 / -2:

- `pkg/ingester/active_queried_series.go` - one identifier changed
  (`runtime.NumCPU()` -> `runtime.GOMAXPROCS(0)`) plus a 3-line explanatory
  comment on the existing worker-count line. `min(..., 4)` cap unchanged.
- `pkg/ingester/active_queried_series_test.go` - new table-driven test
  `TestActiveQueriedSeriesService_NumWorkersFollowsGOMAXPROCS` (saves/restores
  `GOMAXPROCS`; asserts 1->1, 2->1, 4->2, 16->4).
- `CHANGELOG.md` - one `[BUGFIX] Ingester` line mirroring the neighboring
  entry style (trailing `#XXXX` placeholder).

The active queried series service sized its worker pool with
runtime.NumCPU(), which returns the host's logical CPU count and ignores
the CPU cgroup quota. Cortex imports go.uber.org/automaxprocs so that
GOMAXPROCS matches the container's CPU limit, and reads container-aware
parallelism via runtime.GOMAXPROCS(0) elsewhere (for example the resource
monitor's CPU limit and the engine's decoding concurrency default). This
one call site was inconsistent: in a CPU-limited container on a many-core
host it sized the pool off the host cores instead of the CPU budget.

Use runtime.GOMAXPROCS(0) so the pool follows the container CPU quota,
matching the rest of the codebase. The min(..., 4) cap is unchanged. Add
a regression test asserting the worker count follows GOMAXPROCS.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@dosubot dosubot Bot added component/ingester go Pull requests that update Go code type/bug type/tests labels Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant