-
Notifications
You must be signed in to change notification settings - Fork 690
Map user-input session_id to internal session_id to maintain session identity #4523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -196,13 +196,34 @@ def __init__(self): | |
| """Initialize the session manager.""" | ||
|
|
||
| self.sessions = {} | ||
| self.session_id_generator = itertools.count(1) | ||
| self.session_id_generator = itertools.count(0) | ||
| self.request_handle_pool = None | ||
| self.loop = None | ||
| # user_session_id->session_id. If user specifies | ||
| # a session_id when visiting the api_server's endpoint, | ||
| # we map the user_session_id to the session_id to keep | ||
| # session's id globally identical across different requests. | ||
| self.user_session_id_map = {} | ||
| # session_id->user_session_id map. | ||
| self.session_id_map = {} | ||
|
|
||
| def map_user_session_id(self, user_session_id: int) -> int: | ||
| """Map a user_session_id to a session_id.""" | ||
| if user_session_id in self.user_session_id_map: | ||
| raise ValueError(f'User session id {user_session_id} already exists') | ||
| session_id = next(self.session_id_generator) | ||
| self.user_session_id_map[user_session_id] = session_id | ||
| self.session_id_map[session_id] = user_session_id | ||
| return session_id | ||
|
Comment on lines
+210
to
+217
|
||
|
|
||
| def get(self, session_id: int | None = None, create_if_not_exists: bool = True, **kwargs) -> Session | None: | ||
| """Get or create a session.""" | ||
| if not create_if_not_exists: | ||
| return self.sessions.get(session_id, None) | ||
|
lvhan028 marked this conversation as resolved.
|
||
|
|
||
| if session_id is None: | ||
| session_id = next(self.session_id_generator) | ||
|
|
||
| def get(self, session_id: int | None = None, **kwargs) -> Session: | ||
| """Create a new session.""" | ||
| session_id = session_id or next(self.session_id_generator) | ||
| if session_id in self.sessions: | ||
| logger.debug(f'[SessionManager] session {session_id} already exists. Updating...') | ||
| session = self.sessions[session_id] | ||
|
|
@@ -224,17 +245,24 @@ async def async_abort_all(self): | |
| # "abort all" is designed for async RL. The aborted sessions will be no longer used, | ||
| # so we clear the sessions here. | ||
| self.sessions.clear() | ||
| self.user_session_id_map.clear() | ||
| self.session_id_map.clear() | ||
|
|
||
| def has(self, session_id): | ||
| return session_id in self.sessions | ||
|
|
||
| def remove(self, session: Session): | ||
| self.sessions.pop(session.session_id, None) | ||
| user_session_id = self.session_id_map.pop(session.session_id, None) | ||
| if user_session_id is not None: | ||
| self.user_session_id_map.pop(user_session_id, None) | ||
|
|
||
| def clear(self): | ||
| self.sessions.clear() | ||
| self.user_session_id_map.clear() | ||
| self.session_id_map.clear() | ||
| # reset the session id generator | ||
| self.session_id_generator = itertools.count(1) | ||
| self.session_id_generator = itertools.count(0) | ||
|
|
||
| def attach_event_loop(self, loop): | ||
| self.loop = loop | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -104,24 +104,41 @@ class VariableInterface: | |||||||||||||
| allow_terminate_by_client: bool = False | ||||||||||||||
| enable_abort_handling: bool = False | ||||||||||||||
|
|
||||||||||||||
| @staticmethod | ||||||||||||||
| def get_session(session_id: int) -> Session: | ||||||||||||||
| session_mgr = VariableInterface.get_session_manager() | ||||||||||||||
| if session_id == -1: | ||||||||||||||
| @classmethod | ||||||||||||||
| def create_session(cls, user_session_id: int | None = None) -> Session: | ||||||||||||||
| session_mgr = cls.get_session_manager() | ||||||||||||||
| if user_session_id is None or user_session_id == -1: | ||||||||||||||
| # user doesn't input session_id, so we need to generate a new one | ||||||||||||||
| session = session_mgr.get() | ||||||||||||||
| else: | ||||||||||||||
| # find the inside session_id by user_session_id, create a new one | ||||||||||||||
| # if it doesn't exist and update the user_session_id_map | ||||||||||||||
| session_id = session_mgr.map_user_session_id(user_session_id) | ||||||||||||||
| logger.info(f'created session {session_id} for user_session_id {user_session_id}') | ||||||||||||||
|
lvhan028 marked this conversation as resolved.
Outdated
|
||||||||||||||
| session_id = session_mgr.map_user_session_id(user_session_id) | |
| logger.info(f'created session {session_id} for user_session_id {user_session_id}') | |
| session_id = session_mgr.user_session_id_map.get(user_session_id) | |
| if session_id is None: | |
| session_id = session_mgr.map_user_session_id(user_session_id) | |
| logger.info(f'created session {session_id} for user_session_id {user_session_id}') |
Copilot
AI
Apr 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chat_completions_v1 now passes a mapped/internal session via create_session(). Later in this handler the response id is derived from session.session_id, which will now be the internal id (not the user-provided session_id) and can effectively expose internal ids to clients. Consider switching response ids to use the user-provided session id (or a separate request UUID) so internal ids remain an implementation detail, and keep behavior consistent with /v1/completions.
Uh oh!
There was an error while loading. Please reload this page.