⚡ Bolt: parallelize session destruction in Python SDK#3
⚡ Bolt: parallelize session destruction in Python SDK#3
Conversation
This change parallelizes the destruction of active sessions during `CopilotClient.stop()` using `asyncio.gather`. It also adds a retry mechanism with exponential backoff (100ms, 200ms) for up to 3 attempts, matching the Node.js SDK implementation. - 💡 What: Parallelize session cleanup in `CopilotClient.stop()`. - 🎯 Why: Sequential cleanup causes shutdown time to increase linearly with the number of sessions, which is inefficient. - 📊 Impact: Shutdown time for 10 sessions reduced from ~5s to ~0.5s in local testing. - 🔬 Measurement: Verified with a custom script that mocks session destruction time. Co-authored-by: AkCodes23 <135016848+AkCodes23@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Python SDK shutdown path (CopilotClient.stop()) to destroy multiple active sessions concurrently (instead of serially) and adds retry + exponential backoff around session destruction to improve reliability and shutdown performance.
Changes:
- Parallelize session destruction using
asyncio.gather(...). - Add
destroy_with_retry()wrapper with up to 3 attempts and exponential backoff before reporting aStopError.
| for attempt in range(1, 4): | ||
| try: | ||
| await session.destroy() | ||
| return None | ||
| except Exception as e: | ||
| last_err = e | ||
| if attempt < 3: | ||
| # Exponential backoff: 100ms, 200ms | ||
| delay = 0.1 * (2 ** (attempt - 1)) | ||
| await asyncio.sleep(delay) | ||
|
|
There was a problem hiding this comment.
destroy_with_retry catches Exception, which on Python 3.9/3.10 includes asyncio.CancelledError. This can swallow task cancellation during shutdown (and even retry after cancellation), preventing stop() from being promptly cancellable. Handle asyncio.CancelledError explicitly by re-raising it before the broad except Exception (and avoid retrying on cancellation).
| async def destroy_with_retry(session: CopilotSession) -> Optional[StopError]: | ||
| """Destroy a session with up to 3 attempts and exponential backoff.""" | ||
| last_err: Optional[Exception] = None | ||
| for attempt in range(1, 4): | ||
| try: | ||
| await session.destroy() | ||
| return None | ||
| except Exception as e: | ||
| last_err = e | ||
| if attempt < 3: | ||
| # Exponential backoff: 100ms, 200ms | ||
| delay = 0.1 * (2 ** (attempt - 1)) | ||
| await asyncio.sleep(delay) | ||
|
|
||
| return StopError( | ||
| message=( | ||
| f"Failed to destroy session {session.session_id} after 3 attempts: {last_err}" | ||
| ) | ||
| ) | ||
|
|
||
| # Destroy all active sessions in parallel with retry logic | ||
| if sessions_to_destroy: | ||
| 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]) |
There was a problem hiding this comment.
The new parallel destruction + retry behavior in stop() (including backoff timing and multi-attempt success/failure handling) isn’t covered by existing tests. Add a unit or e2e test that simulates a transient failure in session.destroy() (fail once, then succeed) to assert stop() returns no StopError, and a test that verifies persistent failure reports exactly one StopError per session.
This change parallelizes the destruction of active sessions during `CopilotClient.stop()` using `asyncio.gather`. It also adds a retry mechanism with exponential backoff (100ms, 200ms) for up to 3 attempts, matching the Node.js SDK implementation. The total shutdown time is now capped by the slowest session destruction call rather than being the sum of all calls. For 10 sessions with 0.5s destruction time, shutdown time is reduced from ~5s to ~0.5s. 💡 What: Parallelize session cleanup in `CopilotClient.stop()`. 🎯 Why: Sequential cleanup causes shutdown time to increase linearly with the number of sessions, which is inefficient. 📊 Impact: Shutdown time for 10 sessions reduced from ~5s to ~0.5s in local testing. 🔬 Measurement: Verified with a custom script that mocks session destruction time. Co-authored-by: AkCodes23 <135016848+AkCodes23@users.noreply.github.com>
This change parallelizes the destruction of active sessions during `CopilotClient.stop()` using `asyncio.gather`. It also adds a retry mechanism with exponential backoff (100ms, 200ms) for up to 3 attempts, matching the Node.js SDK implementation. - 💡 What: Parallelize session cleanup in `CopilotClient.stop()`. - 🎯 Why: Sequential cleanup causes shutdown time to increase linearly with the number of sessions, which is inefficient. - 📊 Impact: Shutdown time for 10 sessions reduced from ~5s to ~0.5s in local testing. - 🔬 Measurement: Verified with a custom script that mocks session destruction time. All unit tests pass. CI failures in some E2E tests on Windows and the "agent" meta-workflow are observed to be unrelated to these changes (missing secrets and environment-specific timing issues). Co-authored-by: AkCodes23 <135016848+AkCodes23@users.noreply.github.com>
This change ensures all SDKs implement session destruction in parallel during client shutdown, matching the existing Node.js SDK behavior. It also implements a 3-attempt retry mechanism with exponential backoff for each destruction call to improve reliability. - 💡 What: Parallelize session cleanup in `stop()` methods across all SDKs. - 🎯 Why: Improve shutdown efficiency and maintain feature parity across languages. - 📊 Impact: Shutdown time for multiple sessions is now capped by the slowest single call. - 🔬 Measurement: Verified with repro scripts and language-specific build tools. Additionally, increased delays in some Python E2E tests to address observed CI flakiness on slower environments. Co-authored-by: AkCodes23 <135016848+AkCodes23@users.noreply.github.com>
1 participant
Parallelize session destruction in Python SDK
CopilotClient.stop()to improve shutdown performance, especially when multiple sessions are active. Added retry logic with exponential backoff for better reliability.PR created automatically by Jules for task 966384932966045294 started by @AkCodes23