Repository navigation
feat(sessions): add optional pagination to list_sessions - #7799
Open
inamdarmihir wants to merge 1 commit into
Open
inamdarmihir wants to merge 1 commit into
inamdarmihir wants to merge 1 commit into
Conversation
list_sessions returned every session for an app/user, which takes over a minute for ~10k sessions. Add optional page_size and page_token arguments and an optional next_page_token on ListSessionsResponse. Without them the behavior is unchanged. The token is an opaque keyset cursor on the existing (last_update_time, user_id, id) order, scoped to the app_name and user_id it was issued for, so sessions added, updated or removed between pages never cause others to be skipped or repeated. The database and sqlite services apply the cursor and LIMIT in SQL; the in-memory service copies only the returned page. Vertex AI, Redis, MongoDB and Firestore cannot order by update time natively, so they page the sorted list client-side. The web endpoint takes optional page_size/page_token query parameters and returns the next token in the X-Next-Page-Token header, passing them to the session service only when given so services with the old signature keep working. Fixes google#4621
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.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
list_sessionsreturns every session for an app/user with no way to limit the result. The issue reports ~74s with ~10k sessions per app, which makes the sessions endpoint unusable.Solution:
Optional, backward-compatible keyset pagination. With no new arguments the behavior is exactly as before.
list_sessions(..., page_size: Optional[int] = None, page_token: Optional[str] = None)andListSessionsResponse.next_page_token(also onInMemorySessionService.list_sessions_sync).(last_update_time, user_id, id)order, so ties are deterministic and pages stay correct when sessions are added or deleted between calls. It recordsapp_name/user_id; a token from another filter, or a malformed one, raisesValueError. A call fetchespage_size + 1rows, so an exact multiple has no empty trailing page and the last page has no token.page_sizemust be a positive int.DatabaseSessionServiceandSqliteSessionServicepush the cursor condition andLIMITinto SQL (spelled as OR clauses since MSSQL has no row-value comparison); the in-memory service sorts references and copies only the returned page.GET /apps/{app}/users/{user}/sessionsgets optionalpage_sizeandpage_tokenquery parameters, returns the next token in anX-Next-Page-Tokenheader (added to CORSexpose_headers) so the body stays a plain list, and passes the new arguments only when supplied. A service whoselist_sessionshas the old signature gets a 501 only if a client asks for paging; aValueErrorfrom a supplied token becomes a 400.A session updated between pages moves to the end of the order and can appear again later; this is documented and tested. adk-js uses offset paging; I chose cursor paging because offsets skip or repeat rows when data changes.
Testing Plan
Unit Tests:
tests/unittests/sessions/test_list_sessions_pagination.pyruns on the sharedsession_servicefixture (in-memory plain and light-copy, database, sqlite, Redis, MongoDB, per-agent database). Cases: default unchanged; page larger than total; page sizes 1, 2, 3, 4, 7, 8, 100; exact multiple; empty app and unknown user; user filter with paging; invalidpage_size; 12 malformed token shapes; tokens from another app or user filter; sessions added, deleted and updated between pages; equal update times; app/user state still merged. Plus Vertex AI, Firestore and endpoint tests (paging with header, 422/400, empty token, 501 for an old-signature service).Manual End-to-End (E2E) Tests:
Throwaway script paging 23 sessions across three users with page sizes 1, 2, 5, 10, 23, 24 and 100 on the in-memory, database and sqlite services: every session returned exactly once, in the same order as the unpaged listing. Only sqlite was exercised for the SQL path; not run against Postgres, MySQL or MSSQL.
Checklist
Additional context
The change is large because the base signature is shared by every backend. If maintainers prefer, I can split it into the base/in-memory/SQL services first and the other backends and the endpoint afterwards.