Just for benchmarking: add force launch option#93
Open
lukaskratzel wants to merge 1 commit intomainfrom
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChanges introduce session reuse control for ephemeral launches. Changes
Sequence DiagramsequenceDiagram
participant Client
participant RootResource
participant K8sUtil
participant WorkspaceUtil
Client->>RootResource: POST /launch (LaunchRequest with reuseExistingSession)
alt reuseExistingSession = true
RootResource->>K8sUtil: launchEphemeralSession(..., true, ...)
K8sUtil->>K8sUtil: getSessionName(user, app, false)
K8sUtil->>WorkspaceUtil: generateUniqueWorkspaceName(user, app)
WorkspaceUtil->>WorkspaceUtil: append UUID suffix to workspace name
WorkspaceUtil-->>K8sUtil: ws-{epochMillis}-{uuidSuffix}{app}-{user}
else reuseExistingSession = false
RootResource->>K8sUtil: launchEphemeralSession(..., false, ...)
K8sUtil->>K8sUtil: getSessionName(user, app, true)
K8sUtil->>WorkspaceUtil: generateUniqueWorkspaceName(user, app)
WorkspaceUtil->>WorkspaceUtil: append UUID suffix to workspace name
WorkspaceUtil-->>K8sUtil: ws-{epochMillis}-{uuidSuffix}{app}-{user}
end
K8sUtil->>K8sUtil: launchSession with computed name
K8sUtil-->>RootResource: session identifier
RootResource-->>Client: 200 OK
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a benchmarking-oriented option to control whether ephemeral session launches should reuse an existing session or force creation of a fresh one, by threading a new request flag down into the Kubernetes launch logic. Also improves uniqueness of generated workspace names to reduce collisions.
Changes:
- Add
reuseExistingSessionflag toLaunchRequest(defaulttrue) and include it in request logging. - Extend
K8sUtil.launchEphemeralSession(...)with a reuse/force-new parameter and wire it fromRootResource. - Make unique workspace naming more collision-resistant by adding a short UUID suffix.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
java/service/.../session/SessionResource.java |
Updates ephemeral launch call to match new K8sUtil.launchEphemeralSession signature (keeps reuse behavior). |
java/service/.../RootResource.java |
Passes new reuseExistingSession flag through to ephemeral session launches. |
java/service/.../LaunchRequest.java |
Introduces reuseExistingSession request field and logs it via toString(). |
java/service/.../K8sUtil.java |
Adds reuse/force-new parameter to ephemeral session launch and uses it to decide unique vs stable session naming. |
java/common/.../WorkspaceUtil.java |
Adds UUID-based suffix to unique workspace name generation to reduce collisions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return asValidName((WORKSPACE_PREFIX + Instant.now().toEpochMilli() + getWorkspaceDescription(appDefinitionName) | ||
| + "-" + user).toLowerCase(), WORKSPACE_NAME_LIMIT); | ||
| String suffix = UUID.randomUUID().toString().substring(0, 8); | ||
| return asValidName((WORKSPACE_PREFIX + Instant.now().toEpochMilli() + "-" + suffix |
Comment on lines
98
to
102
| if (request.isEphemeral()) { | ||
| info(correlationId, "Launching ephemeral session " + request); | ||
| return k8sUtil.launchEphemeralSession(correlationId, request.appDefinition, user, request.timeout, | ||
| request.env); | ||
| return k8sUtil.launchEphemeralSession(correlationId, request.appDefinition, user, | ||
| request.reuseExistingSession, request.timeout, request.env); | ||
| } |
| SessionSpec sessionSpec = new SessionSpec(getSessionName(user, appDefinition, false), appDefinition, user); | ||
| public String launchEphemeralSession(String correlationId, String appDefinition, String user, | ||
| boolean reuseExistingSession, int timeout, EnvironmentVars env) { | ||
| SessionSpec sessionSpec = new SessionSpec(getSessionName(user, appDefinition, !reuseExistingSession), |
| @Schema(description = "If true no workspace will be created for the session.", required = false) | ||
| public boolean ephemeral; | ||
|
|
||
| @Schema(description = "If false, ephemeral launches will always create a fresh session instead of reusing an existing one for the same user and app definition.", required = false) |
lukaskratzel
force-pushed
the
benchmark/force-launch-option
branch
from
6f52f10 to
8a76748
Compare
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.
Summary by CodeRabbit