Skip to content

Commit e0f3104

Browse files
authored
fix: slow workspace list refresh when many workspaces are stopped (#328)
A customer reported that refreshing the workspace list could take around ten seconds. The client logs showed that most of that time was spent expanding the auth header command: stopped workspaces do not include their resources in the /workspaces response, so we were issuing a separate resource lookup for each stopped workspace just to recover its agents, and every one of those HTTP calls paid the header command expansion cost (roughly half a second per call — about 9.7 seconds for the 21 stopped workspaces in the customer's search). Instead of trying to make those calls faster, we questioned why they were needed at all. Agents only matter when there is something to connect to, and nothing can connect to a stopped workspace: the SSH configuration is only useful for running workspaces, and the agent list arrives for free in the /workspaces response the moment a workspace is running. So stopped (and otherwise not-running) workspaces are now registered as workspace-only environments, identified by workspace name alone, with no resource lookup at all. When a workspace comes up, the poller replaces the workspace-only row with one row per agent and regenerates the SSH configuration, which now covers running workspaces only. While reviewing the features that interact with stopped workspaces I found and fixed three problems this change would have introduced: - deep links (URI handling) became racy: the agent environment and its SSH config entry now exist only after the poller observes the running workspace, so a fixed two-second delay was no longer enough. The protocol handler now nudges the poller and waits for the environment to actually appear (with a timeout) before showing the environment page and launching the IDE. - an environment without an agent used to hand Toolbox an SSH contents view with an empty host. It now returns a manual contents view with empty IDE and project lists, so Toolbox never attempts to connect to a blank host. - Toolbox closes environments that disappear from the provider list by cancelling their wrapper scope without ever firing the AfterDisconnectHook. This one took a lot of time to figure out and was only possible by verifying against the Toolbox bytecode. Since stop/start transitions now replace environment instances, a connected environment leaked its network metrics poll job. The provider now disposes dropped instances explicitly, both during poll refreshes and on close/logout. Two known issues remain and I think we can consider them acceptable: (1) the environment id changes across a stop/start cycle, so an environment page left open while the workspace stops can go throw a weird error about Toolbox not being able to find it; (2) a workspace that goes from running and ssh connected to stopped and then back to running, no longer auto-connects - resolves DEVEX-372 - resolves DEVEX-374
1 parent bd1ab0e commit e0f3104

12 files changed

Lines changed: 173 additions & 192 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
- snackbar dismissal no longer cancels the calling coroutine, so error popups during URI handling don't leave the page
1313
stuck in a busy state
14+
- faster workspace list refresh: agents are no longer fetched for stopped workspaces
1415

1516
### Changed
1617

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,9 @@ Changing the search or a dropdown refreshes the workspace list and regenerates S
485485
workspace
486486
set.
487487
With wildcard SSH enabled, the generated block uses the deployment wildcard host pattern. With wildcard SSH disabled,
488-
the generated block contains entries for the currently resolved workspace/agent pairs.
488+
the generated block contains entries for the currently resolved workspace/agent pairs. Agents are resolved only for
489+
running workspaces, so stopped workspaces have no SSH entries until they are started and picked up by the next
490+
workspace refresh.
489491

490492
SSH hostnames include the workspace owner so workspaces with the same name owned by different users remain distinct.
491493
With wildcard SSH enabled, the SSH config contains a deployment-wide `Host coder-jetbrains-toolbox-<host>--*` entry, and

src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import com.coder.toolbox.util.waitForFalseWithTimeout
1313
import com.coder.toolbox.util.withPath
1414
import com.coder.toolbox.views.Action
1515
import com.coder.toolbox.views.CoderDelimiter
16+
import com.coder.toolbox.views.EmptyWorkspaceContentView
1617
import com.coder.toolbox.views.EnvironmentView
1718
import com.jetbrains.toolbox.api.localization.LocalizableString
1819
import com.jetbrains.toolbox.api.remoteDev.AfterDisconnectHook
@@ -28,6 +29,7 @@ import com.squareup.moshi.Moshi
2829
import kotlinx.coroutines.CoroutineName
2930
import kotlinx.coroutines.Dispatchers
3031
import kotlinx.coroutines.Job
32+
import kotlinx.coroutines.channels.Channel
3133
import kotlinx.coroutines.delay
3234
import kotlinx.coroutines.flow.MutableStateFlow
3335
import kotlinx.coroutines.flow.StateFlow
@@ -42,21 +44,25 @@ import kotlin.time.Duration.Companion.seconds
4244

4345
private val POLL_INTERVAL = 5.seconds
4446

47+
private fun environmentId(workspace: Workspace, agent: WorkspaceAgent?): String =
48+
agent?.let { "${workspace.name}.${it.name}" } ?: workspace.name
49+
4550
/**
46-
* Represents an agent and workspace combination.
51+
* Represents a workspace, or a workspace and agent combination when an agent is available.
4752
*
4853
* Used in the environment list view.
4954
*/
5055
class CoderRemoteEnvironment(
5156
private val context: CoderToolboxContext,
5257
internal var client: CoderRestClient,
5358
internal var cli: CoderCLIManager,
59+
private val workspaceRefreshTrigger: Channel<Boolean>,
5460
private var workspace: Workspace,
55-
private var agent: WorkspaceAgent,
56-
) : RemoteProviderEnvironment("${workspace.name}.${agent.name}"), BeforeConnectionHook, AfterDisconnectHook {
61+
private var agent: WorkspaceAgent?,
62+
) : RemoteProviderEnvironment(environmentId(workspace, agent)), BeforeConnectionHook, AfterDisconnectHook {
5763
private var environmentStatus = WorkspaceAndAgentStatus.from(workspace, agent)
5864

59-
override var name: String = "${workspace.name}.${agent.name}"
65+
override var name: String = environmentId(workspace, agent)
6066
private var isConnected: MutableStateFlow<Boolean> = MutableStateFlow(false)
6167
override val connectionRequest: MutableStateFlow<Boolean> = MutableStateFlow(false)
6268

@@ -79,12 +85,12 @@ class CoderRemoteEnvironment(
7985
refreshAvailableActions()
8086
}
8187

82-
fun asPairOfWorkspaceAndAgent(): Pair<Workspace, WorkspaceAgent> = Pair(workspace, agent)
88+
fun toWorkspaceAgentPairOrNull(): Pair<Workspace, WorkspaceAgent>? = agent?.let { Pair(workspace, it) }
8389

8490
private fun refreshAvailableActions() {
8591
val actions = mutableListOf<ActionDescription>()
8692
context.logger.debug("Refreshing available actions for workspace $id with status: $environmentStatus")
87-
if (environmentStatus.canStop()) {
93+
if (environmentStatus.canStop() && agent != null) {
8894
actions.add(Action(context, "Open web terminal") {
8995
context.logger.debug("Launching web terminal for $id...")
9096
context.desktop.browse(client.url.withPath("/${workspace.ownerName}/$name/terminal").toString()) {
@@ -122,13 +128,15 @@ class CoderRemoteEnvironment(
122128
context.logger.debug("Updating and starting $id...")
123129
val build = client.updateWorkspace(workspace)
124130
update(workspace.copy(latestBuild = build), agent)
131+
workspaceRefreshTrigger.trySend(true)
125132
})
126133
} else {
127134
actions.add(Action(context, "Start") {
128135
context.logger.debug("Starting $id... ")
129136
context.cs
130137
.launch(CoroutineName("Start Workspace Action CLI Runner") + Dispatchers.IO) {
131138
cli.startWorkspace(workspace.ownerName, workspace.name)
139+
workspaceRefreshTrigger.trySend(true)
132140
}
133141
// cli takes 15 seconds to move the workspace in queueing/starting state
134142
// while the user won't see anything happening in TBX after start is clicked
@@ -145,6 +153,7 @@ class CoderRemoteEnvironment(
145153
context.logger.debug("Updating and re-starting $id...")
146154
val build = client.updateWorkspace(workspace)
147155
update(workspace.copy(latestBuild = build), agent)
156+
workspaceRefreshTrigger.trySend(true)
148157
}
149158
)
150159
}
@@ -204,6 +213,9 @@ class CoderRemoteEnvironment(
204213
override fun getAfterDisconnectHooks(): List<AfterDisconnectHook> = listOf(this)
205214

206215
override fun beforeConnection() {
216+
if (agent == null) {
217+
return
218+
}
207219
context.logger.info("Connecting to $id...")
208220
isConnected.update { true }
209221
context.settingsStore.updateAutoConnect(this.id, true)
@@ -213,8 +225,9 @@ class CoderRemoteEnvironment(
213225
private fun pollNetworkMetrics(): Job = context.cs.launch(CoroutineName("Network Metrics Poller")) {
214226
context.logger.info("Starting the network metrics poll job for $id")
215227
while (isActive) {
228+
val currentAgent = agent ?: break
216229
context.logger.debug("Searching SSH command's PID for workspace $id...")
217-
val pid = proxyCommandHandle.findByWorkspaceAndAgent(workspace, agent)
230+
val pid = proxyCommandHandle.findByWorkspaceAndAgent(workspace, currentAgent)
218231
if (pid == null) {
219232
context.logger.debug("No SSH command PID was found for workspace $id")
220233
delay(POLL_INTERVAL)
@@ -244,6 +257,20 @@ class CoderRemoteEnvironment(
244257

245258
private fun File.doesNotExists(): Boolean = !this.exists()
246259

260+
/**
261+
* Cancels background work when the provider drops this environment from its
262+
* list (e.g. a running workspace stopped and is replaced by a workspace-only
263+
* environment with a different id).
264+
*
265+
* Toolbox reacts to the removal by closing its environment wrapper, which only
266+
* cancels the wrapper's own coroutine scope and never invokes the
267+
* [AfterDisconnectHook], so the network metrics poller must be stopped here.
268+
*/
269+
fun dispose() {
270+
pollJob?.cancel()
271+
isConnected.update { false }
272+
}
273+
247274
override fun afterDisconnect(isManual: Boolean) {
248275
context.logger.info("Stopping the network metrics poll job for $id")
249276
pollJob?.cancel()
@@ -259,20 +286,20 @@ class CoderRemoteEnvironment(
259286
/**
260287
* Update the workspace/agent status to the listeners, if it has changed.
261288
*/
262-
/**
263-
* Update the workspace/agent status to the listeners, if it has changed.
264-
*/
265-
fun update(workspace: Workspace, agent: WorkspaceAgent) {
289+
fun update(workspace: Workspace, agent: WorkspaceAgent?) {
266290
if (this.workspace.latestBuild == workspace.latestBuild) {
267291
return
268292
}
269293
this.workspace = workspace
270294
this.agent = agent
295+
name = environmentId(workspace, agent)
271296

272297
// workspace&agent status can be different from "environment status"
273298
// which is forced to queued state when a workspace is scheduled to start
274299
updateStatus(WorkspaceAndAgentStatus.from(workspace, agent))
275-
context.connectionMonitoringService.checkConnectionStatus(workspace, agent)
300+
if (agent != null) {
301+
context.connectionMonitoringService.checkConnectionStatus(workspace, agent)
302+
}
276303

277304
// we have to regenerate the action list in order to force a redraw
278305
// because the actions don't have a state flow on the enabled property
@@ -285,20 +312,33 @@ class CoderRemoteEnvironment(
285312
state.update {
286313
environmentStatus.toRemoteEnvironmentState(context)
287314
}
288-
context.logger.info("Overall status for workspace $id is $environmentStatus. Workspace status: ${workspace.latestBuild.status}, agent status: ${agent.status}, agent lifecycle state: ${agent.lifecycleState}, login before ready: ${agent.loginBeforeReady}")
315+
context.logger.info(
316+
"Overall status for workspace $id is $environmentStatus. " +
317+
"Workspace status: ${workspace.latestBuild.status}, " +
318+
"agent status: ${agent?.status}, " +
319+
"agent lifecycle state: ${agent?.lifecycleState}, " +
320+
"login before ready: ${agent?.loginBeforeReady}"
321+
)
289322
}
290323

291324
/**
292325
* The contents are provided by the SSH view provided by Toolbox, all we
293-
* have to do is provide it a host name.
326+
* have to do is provide it a host name. Workspaces without a resolved agent
327+
* have no host to connect to yet, so they get an empty contents view instead.
294328
*/
295-
override suspend fun getContentsView(): EnvironmentContentsView = EnvironmentView(
296-
context,
297-
client.url,
298-
cli,
299-
workspace,
300-
agent
301-
)
329+
override suspend fun getContentsView(): EnvironmentContentsView {
330+
val envAgent = agent ?: run {
331+
context.logger.info("No agent is available for $id yet, providing an empty contents view")
332+
return EmptyWorkspaceContentView
333+
}
334+
return EnvironmentView(
335+
context,
336+
client.url,
337+
cli,
338+
workspace,
339+
envAgent
340+
)
341+
}
302342

303343
/**
304344
* Automatically launches the SSH connection if the workspace is visible, is ready and there is no

src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,12 @@ class CoderRemoteProvider(
100100
}
101101
}
102102
}
103-
private val linkHandler = CoderProtocolHandler(context, IdeFeedManager(context))
104-
105103
override val loadingEnvironmentsDescription: LocalizableString = context.i18n.ptrl("Loading workspaces...")
106104
override val environments: MutableStateFlow<LoadableState<List<CoderRemoteEnvironment>>> = MutableStateFlow(
107105
LoadableState.Loading
108106
)
107+
private val linkHandler =
108+
CoderProtocolHandler(context, IdeFeedManager(context), workspaceRefreshTrigger, environments)
109109
private val accountDropdownField = dropDownFactory(context.i18n.pnotr("")) {
110110
logout()
111111
context.envPageManager.showPluginEnvironmentsPage(false)
@@ -133,10 +133,14 @@ class CoderRemoteProvider(
133133
return@launch
134134
}
135135

136+
// Toolbox closes removed environments without firing their
137+
// disconnect hooks, so stop their background work before dropping them.
138+
lastEnvironments.filter { it !in resolvedEnvironments }.forEach { it.dispose() }
139+
136140
// Reconfigure if environments changed.
137141
if (lastEnvironments.size != resolvedEnvironments.size || lastEnvironments != resolvedEnvironments) {
138142
context.logger.info("Workspaces have changed, reconfiguring CLI: $resolvedEnvironments")
139-
cli.configSsh(resolvedEnvironments.map { it.asPairOfWorkspaceAndAgent() }.toSet())
143+
cli.configSsh(resolvedEnvironments.mapNotNull { it.toWorkspaceAgentPairOrNull() }.toSet())
140144
}
141145

142146
environments.update {
@@ -181,7 +185,7 @@ class CoderRemoteProvider(
181185
sshConfigTrigger.onReceive { shouldTrigger ->
182186
if (shouldTrigger) {
183187
context.logger.debug("workspace poller waked up because it should reconfigure the ssh configurations")
184-
cli.configSsh(lastEnvironments.map { it.asPairOfWorkspaceAndAgent() }.toSet())
188+
cli.configSsh(lastEnvironments.mapNotNull { it.toWorkspaceAgentPairOrNull() }.toSet())
185189
}
186190
}
187191
workspaceRefreshTrigger.onReceive { shouldTrigger ->
@@ -203,8 +207,8 @@ class CoderRemoteProvider(
203207
* Resolves workspace agents into remote environments.
204208
*
205209
* For each workspace:
206-
* - If running, uses agents from the latest build resources
207-
* - If not running, fetches resources separately
210+
* - If running, uses agents from the latest build resources.
211+
* - If not running, creates a workspace-only environment without resolving agents.
208212
*
209213
* @return a sorted list of resolved remote environments
210214
*/
@@ -224,23 +228,23 @@ class CoderRemoteProvider(
224228
}
225229
coderHeaderPage.resetError()
226230
return workspaces.flatMap { ws ->
227-
// Agents are not included in workspaces that are off
228-
// so fetch them separately.
229-
val resources = when (ws.latestBuild.status) {
230-
WorkspaceStatus.RUNNING -> ws.latestBuild.resources
231-
else -> emptyList()
232-
}.ifEmpty {
233-
client.resources(ws)
231+
if (ws.latestBuild.status != WorkspaceStatus.RUNNING) {
232+
return@flatMap listOf(
233+
lastEnvironments.firstOrNull { it.id == ws.name }
234+
?.also { it.update(ws, null) }
235+
?: CoderRemoteEnvironment(context, client, cli, workspaceRefreshTrigger, ws, null)
236+
)
234237
}
235-
resources
238+
239+
ws.latestBuild.resources
236240
.flatMap { it.agents ?: emptyList() }
237241
.distinctBy { it.name }
238242
.map { agent ->
239243
lastEnvironments.firstOrNull { it.id == "${ws.name}.${agent.name}" }
240244
?.also {
241245
// If we have an environment already, update that.
242246
it.update(ws, agent)
243-
} ?: CoderRemoteEnvironment(context, client, cli, ws, agent)
247+
} ?: CoderRemoteEnvironment(context, client, cli, workspaceRefreshTrigger, ws, agent)
244248
}
245249

246250
}.sortedBy { it.id }
@@ -288,6 +292,7 @@ class CoderRemoteProvider(
288292
softClose()
289293
client = null
290294
cli = null
295+
lastEnvironments.forEach { it.dispose() }
291296
lastEnvironments.clear()
292297
environments.value = LoadableState.Value(emptyList())
293298
isInitialized.update { false }
@@ -590,7 +595,11 @@ class CoderRemoteProvider(
590595
onTokenRefreshed = ::onTokenRefreshed,
591596
)
592597
} catch (ex: Exception) {
593-
context.logAndShowError("Error encountered while setting up Coder", "Failed to set up Coder", ex)
598+
context.logAndShowError(
599+
"Error encountered while setting up Coder",
600+
"Failed to set up Coder: ${ex.message}",
601+
ex
602+
)
594603
} finally {
595604
firstRun = false
596605
}

src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import com.coder.toolbox.sdk.v2.models.User
2121
import com.coder.toolbox.sdk.v2.models.Workspace
2222
import com.coder.toolbox.sdk.v2.models.WorkspaceBuild
2323
import com.coder.toolbox.sdk.v2.models.WorkspaceBuildReason
24-
import com.coder.toolbox.sdk.v2.models.WorkspaceResource
2524
import com.coder.toolbox.sdk.v2.models.WorkspaceTransition
2625
import com.coder.toolbox.util.ReloadableTlsContext
2726
import com.coder.toolbox.views.state.CoderOAuthSessionContext
@@ -202,31 +201,6 @@ open class CoderRestClient(
202201
}
203202
}
204203

205-
/**
206-
* Retrieves resources for the specified workspace. The workspaces response
207-
* does not include agents when the workspace is off so this can be used to
208-
* get them instead, just like `coder config-ssh` does (otherwise we risk
209-
* removing hosts from the SSH config when they are off).
210-
* @throws [APIResponseException].
211-
*/
212-
suspend fun resources(workspace: Workspace): List<WorkspaceResource> {
213-
val resourcesResponse = callWithRetry {
214-
retroRestClient.templateVersionResources(workspace.latestBuild.templateVersionID)
215-
}
216-
if (!resourcesResponse.isSuccessful) {
217-
throw APIResponseException(
218-
"retrieve resources for ${workspace.name}",
219-
url,
220-
resourcesResponse.code(),
221-
resourcesResponse.parseErrorBody(moshi)
222-
)
223-
}
224-
225-
return requireNotNull(resourcesResponse.body()) {
226-
"Successful response returned null body or workspace resources"
227-
}
228-
}
229-
230204
suspend fun buildInfo(): BuildInfo {
231205
val buildInfoResponse = callWithRetry { retroRestClient.buildInfo() }
232206
if (!buildInfoResponse.isSuccessful) {

src/main/kotlin/com/coder/toolbox/sdk/v2/CoderV2RestFacade.kt

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import com.coder.toolbox.sdk.v2.models.Template
77
import com.coder.toolbox.sdk.v2.models.User
88
import com.coder.toolbox.sdk.v2.models.Workspace
99
import com.coder.toolbox.sdk.v2.models.WorkspaceBuild
10-
import com.coder.toolbox.sdk.v2.models.WorkspaceResource
1110
import com.coder.toolbox.sdk.v2.models.WorkspacesResponse
1211
import retrofit2.Response
1312
import retrofit2.http.Body
@@ -69,8 +68,4 @@ interface CoderV2RestFacade {
6968
@Path("templateID") templateID: UUID,
7069
): Response<Template>
7170

72-
@GET("api/v2/templateversions/{templateID}/resources")
73-
suspend fun templateVersionResources(
74-
@Path("templateID") templateID: UUID,
75-
): Response<List<WorkspaceResource>>
7671
}

0 commit comments

Comments
 (0)