Count requests still setting up in the eviction autoscaler metric - #921
Draft
sduchesneau wants to merge 2 commits into
Draft
Count requests still setting up in the eviction autoscaler metric#921sduchesneau wants to merge 2 commits into
sduchesneau wants to merge 2 commits into
Conversation
The evictor counted only requests in its manager map, which they enter after session acquisition and store setup. Admission and substreams_active_requests count them from arrival, so the autoscaler metric could read below the plain count it is meant to replace.
…caler-metric-count
🔍 Vulnerabilities of
|
| digest | sha256:fddab28041530bb68c1f15983e9f63da11538c3f0a71857a1008d67fa2a22b0d |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 124 MB |
| packages | 380 |
📦 Base Image ubuntu:24.04
| also known as |
|
| digest | sha256:1e0a86e57d247923571b75e0aaf48a1449cf8c543d51fb3e07a4a7d7bfa79316 |
| vulnerabilities |   |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
substreams_tier1_effective_active_requestscould read belowsubstreams_active_requests, the metric it is meant to replace as the horizontal autoscaler input. It is amax()of the active-request count and the CPU-derived count, so that should not be possible.The two were counting different sets of requests.
substreams_active_requestsincrements early inblocks(), while a request only enters the evictor's manager map after session acquisition, the deterministic-error check against the cache store and store cloning. Every request still in setup was counted by one and invisible to the other. Teardown has the same shape: the deferredmanager.Removeis registered later than the deferredTier1ActiveRequests.Dec(), so it runs first.The evictor now takes its active-request count from the same accessor admission uses (
getActiveRequestCount), through aCountActiveRequestsWithhook, falling back to the manager map when unset. The manager map is still the source of eviction candidates — a request in setup has no stats to sample and nothing worth cancelling.Effect: a pod with requests queued in setup no longer looks emptier to the autoscaler than it is, and the eviction metric agrees with the soft and hard admission limits about how many requests exist.