Stop the server from accumulating per-request and per-session state - #13756
Open
abidlabs wants to merge 7 commits into
Open
Stop the server from accumulating per-request and per-session state#13756abidlabs wants to merge 7 commits into
abidlabs wants to merge 7 commits into
Conversation
Four structures grew for the life of the process. The first is the one behind #11602: memory that rises with every interaction and never comes back down. `Queue.event_ids_to_events` was never pruned when an event finished -- only when one was removed from the queue unrun. Each retained `Event` pins the `fastapi.Request` it came from and the request payload, so the process kept a copy of every request it had ever served. Entries are now dropped when the event is done. Nothing reads one afterwards: the `/stream/{event_id}` routes only feed an event that is still running, and `/queue/data/{event_id}` already falls back to treating the id as the session hash, which is what it is for a client that sent no session hash. `Queue.event_analytics` was also unbounded, and `compute_analytics_summary` builds a DataFrame over the whole of it, so serving requests got steadily slower as well as heavier. It now keeps the most recent `GRADIO_ANALYTICS_MAX_EVENTS` (10k). Two details this required: the monitoring dashboard holds a reference to that exact dict, so entries are evicted in place rather than by rebinding it; and the summary's cache trigger compared `len()` against a high-water mark, which would stop firing once the history filled, so it now counts events seen. `StateHolder` deleted expired `gr.State` values but never the session itself, so every session ever opened left behind a `SessionState` -- a copy of the blocks config plus a config dict per component -- and an entry in `time_last_used`, which was not even bounded by `state_session_capacity`. A closed session is now forgotten once its state has outlived `STATE_TTL_WHEN_CLOSED`, and evicting over capacity no longer orphans its `time_last_used` entry. `Blocks.pending_streams` is a `defaultdict`, and three public `/stream/...` routes took `session_hash` from the URL and indexed straight into it, so any request naming an unknown session inserted a permanent entry. Those reads no longer create entries, and a session's streams are dropped when its tab closes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
🪼 branch checks and previews
Install Gradio from this PR pip install https://huggingface.co/buckets/gradio/pypi-previews/resolve/0a72a482aaeecec47dee3dbeb069091fb2db0cbb/gradio-6.24.0-py3-none-any.whlInstall Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@0a72a482aaeecec47dee3dbeb069091fb2db0cbb#subdirectory=client/python"Import Gradio JS Client from this PR via CDN import { Client } from "https://huggingface.co/buckets/gradio/npm-previews/resolve/0a72a482aaeecec47dee3dbeb069091fb2db0cbb/browser.js"; |
Two more structures that grew once per request, both found by taking a census of every container reachable from the app and diffing it across two identical batches of work rather than by reading the code. `Queue._asyncio_tasks` held every `process_events` task the queue had ever started, and was only emptied by `_cancel_asyncio_tasks()` at shutdown. Its purpose is to have something to cancel, and a finished task is not something to cancel, so it is now a set each task drops itself from when it completes. `reset_iterators` added the event id to `App.iterators_to_reset` on the way out. That set is there so a job being cancelled does not restore its iterator and overwrite the state, which is why the `/reset` route records it -- that runs while the job is still going. `reset_iterators` has one caller, in the event's `finally`, so an entry it added could never be read by anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
🦄 change detectedThis Pull Request includes changes to the following packages.
|
Fixes the CI failure the first commit caused. `pending_event_ids_session` keeps a set of event ids per session that was only pruned for events removed from the queue unrun, so it held ids of finished events; the unload handler walks that set and indexed straight into `event_ids_to_events`, which raised `KeyError` once finished events stopped being kept. The id is now discarded from both when the event ends, and the lookups that could meet an id for an event that is already gone use `.get`. That set was a per-request leak of its own, inside a container the growth census only measured the outer length of. Also: inline comments removed, and `gradio.profiling` hoisted to the module imports. `from gradio.components import State` in `state_holder.py` has to stay where it is -- at module scope it is a circular import. Adds the growth-census test discussed on the PR: it walks every container reachable from the queue, blocks, state holder and app, and asserts none is larger after a second identical batch of requests on the same session. It fails on main naming all four request-scoped leaks, and it is what found `_asyncio_tasks`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses server-side memory growth reported in #11602 by ensuring per-request and per-session data structures don’t retain completed requests/sessions indefinitely across the lifetime of the process.
Changes:
- Evicts finished queue events/tasks and bounds queue analytics retention to prevent per-request growth.
- Ensures closed session state is eventually dropped and that auxiliary session bookkeeping doesn’t accumulate.
- Avoids retaining/looking up missing stream/event entries and proactively ends streams on disconnect.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
gradio/queueing.py |
Cleans up per-event state after completion, converts task tracking to a set, and bounds analytics retention. |
gradio/state_holder.py |
Drops closed sessions after TTL and keeps time_last_used consistent with evictions. |
gradio/routes.py |
Makes stream/event lookups resilient (no KeyError), ends pending streams on disconnect, and avoids pinning events. |
gradio/route_utils.py |
Avoids KeyError when looking up pending streams for a session. |
test/test_queueing.py |
Adds regression tests for queue state not accumulating and analytics bounding behavior. |
test/test_state_holder.py |
Adds regression tests for session retention/expiry and capacity eviction bookkeeping. |
test/test_no_growth.py |
Adds a regression test asserting container sizes don’t grow across identical request batches. |
.changeset/open-goats-build.md |
Adds a changeset file (may conflict with repo process that auto-generates these). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1
to
+5
| --- | ||
| "gradio": patch | ||
| --- | ||
|
|
||
| fix:Stop the server from accumulating per-request and per-session state |
Member
Author
There was a problem hiding this comment.
Disagreeing with this one: I didn't write that file, gradio-pr-bot did.
$ git log --format="%h %an %s" -- .changeset/open-goats-build.md
29eb1aff9 gradio-pr-bot add changeset
That is the Action described in the guidance doing its job — and AGENTS.md #7 says the bot "leaves an existing changeset alone", which is why writing one by hand is the thing to avoid. Deleting this one would remove the entry generated from the PR title, and the bot would just add it back on the next push.
The other five comments are addressed in the follow-up commit.
Comment on lines
+160
to
+162
| self.ANALYTICS_MAX_EVENTS = int( | ||
| os.getenv("GRADIO_ANALYTICS_MAX_EVENTS", "10000") | ||
| ) |
Comment on lines
190
to
194
| def compute_analytics_summary(self, event_analytics): | ||
| if ( | ||
| len(event_analytics) - self.event_count_at_last_cache | ||
| self.events_recorded - self.event_count_at_last_cache | ||
| >= self.ANAYLTICS_CACHE_FREQUENCY | ||
| ): |
Comment on lines
475
to
+478
| } | ||
| self.events_recorded += 1 | ||
| while len(self.event_analytics) > self.ANALYTICS_MAX_EVENTS: | ||
| self.event_analytics.pop(next(iter(self.event_analytics))) |
Comment on lines
+399
to
+401
| assert demo._queue.cached_event_analytics_summary["functions"]["lambda"][ | ||
| "total_requests" | ||
| ] |
Comment on lines
+64
to
+68
| grew = { | ||
| key: (before[key], after.get(key, 0)) | ||
| for key in before | ||
| if after.get(key, 0) > before[key] | ||
| } |
Popping the session's entry once its set emptied changed something three callers read directly with `[session_hash]`, including the SSE loop that decides when to close a stream and an existing test. Discarding the id is what fixes the growth; the key itself is per session, not per request, and `clean_events` already removes it. Not worth the blast radius. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five points from the Copilot review: `GRADIO_ANALYTICS_MAX_EVENTS` is clamped to at least 1. At zero or negative the trim loop emptied the dict and then raised `StopIteration` from `pop(next(iter(...)))`. A non-numeric value still raises at startup, the same as the `GRADIO_ANALYTICS_CACHE_FREQUENCY` beside it. `compute_analytics_summary` ran on a worker thread and iterated the live dict, which the event loop mutates; trimming made that more likely, not less. It now takes a snapshot of the values, taken on the event loop, and returns the cached summary rather than calling `groupby` when there is nothing in it. `total_requests` no longer undercounts once the window fills. It comes from a per-function lifetime counter, so bounding the history costs the percentiles their older samples but leaves the totals exact. The bounded-analytics test asserts the exact window size and total instead of truthiness, and the growth census compares the union of both censuses so a container that only appears in the second one still counts as growth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were causing CI failures, and neither was carrying its weight. Dropping a closed session's `SessionState` looked safe because it only happens after `STATE_TTL_WHEN_CLOSED`, but the e2e launcher sets `GRADIO_IS_E2E_TEST`, which makes that one second. A second after any unload beacon the session's `config_values` were gone, so the eleven functional failures were mostly `state_change` tests. `session_data` is already bounded by `state_session_capacity`; the structure that actually grew without limit was `time_last_used`, and pruning it when capacity evicts a session is the whole fix. Reducing what a closed session holds needs its own change. Discarding a finished event's id from `pending_event_ids_session` is also reverted. That set is how the `/queue/data` loop counts the completion messages it still has to deliver -- when it empties, the loop closes the stream -- so draining it from the event's `finally` risks closing a stream while another event's output is still queued behind it. What remains is the part that was measured: an event's `Request` and payload are no longer pinned after it finishes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Fixes #11602.
What was leaking
Four server-side structures grew for the life of the process. The first is the one behind #11602 — memory that rises with every interaction and never comes back down.
Queue.event_ids_to_eventsfastapi.Requestand the request payloadQueue.event_analyticsStateHolder.session_data/.time_last_usedBlocks.pending_streamsMediaStreamsMeasured on a 5-component app driven over its own API, 200 KB per call, one fresh session per 25 calls: