-
Notifications
You must be signed in to change notification settings - Fork 0
⚡ Bolt: Parallelize session destruction in Python SDK #2
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 all commits
fced5bf
13398f6
b1ba28c
cfa0b1c
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 |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| ## 2025-05-15 - [Sequential session destruction in SDKs] | ||
| **Learning:** All Copilot SDKs (Node.js, Python, Go, .NET) were initially implementing session destruction sequentially during client shutdown. This leads to a linear increase in shutdown time as the number of active sessions grows, especially when individual destructions involve retries and backoff. | ||
| **Action:** Parallelize session cleanup using language-specific concurrency primitives (e.g., `Promise.all` in Node.js, `asyncio.gather` in Python, `Task.WhenAll` in .NET, or WaitGroups/Channels in Go) to ensure shutdown time remains constant and minimal. | ||
| ## 2026-02-07 - [Python SDK] Parallelize Session Destruction | ||
| **Learning:** Sequential cleanup of network-bound resources (like JSON-RPC sessions) leads to (N)$ shutdown time. Parallelizing with `asyncio.gather` reduces it to (1)$ relative to session count. | ||
| **Action:** Always check cleanup/stop methods for sequential IO and parallelize where safe. Implement retry logic for cleanup to match robust SDK patterns. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -313,13 +313,29 @@ async def stop(self) -> list["StopError"]: | |
| sessions_to_destroy = list(self._sessions.values()) | ||
| self._sessions.clear() | ||
|
|
||
| for session in sessions_to_destroy: | ||
| try: | ||
| await session.destroy() | ||
| except Exception as e: | ||
| errors.append( | ||
| StopError(message=f"Failed to destroy session {session.session_id}: {e}") | ||
| async def destroy_with_retry(session: CopilotSession) -> Optional[StopError]: | ||
| last_error: Optional[Exception] = None | ||
| # Try up to 3 times with exponential backoff (match Node.js SDK) | ||
| for attempt in range(1, 4): | ||
| try: | ||
| await session.destroy() | ||
| return None | ||
| except Exception as e: | ||
| last_error = e | ||
| if attempt < 3: | ||
| # Exponential backoff: 100ms, 200ms | ||
| await asyncio.sleep(0.1 * (2 ** (attempt - 1))) | ||
|
|
||
| return StopError( | ||
| message=( | ||
| f"Failed to destroy session {session.session_id} after 3 attempts: {last_error}" | ||
| ) | ||
| ) | ||
|
|
||
| # Destroy all active sessions in parallel to ensure shutdown time is | ||
| # independent of the number of active sessions. | ||
| results = await asyncio.gather(*(destroy_with_retry(s) for s in sessions_to_destroy)) | ||
| errors.extend([r for r in results if r is not None]) | ||
|
Comment on lines
+335
to
+338
|
||
|
|
||
| # Close client | ||
| if self._client: | ||
|
|
||
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.
The Bolt note uses malformed big-O notation: "(N)$" and "(1)$" look like accidental LaTeX remnants and read incorrectly. Please change these to standard notation (e.g.,
O(N)andO(1)) so the guidance is unambiguous.