Skip to content

Commit 7c64008

Browse files
committed
refactor: simplify workspace start status management
Current approach with a secondary poll loop that handles the start action of a workspace is overengineered. Basically the problem is the CLI takes too long before moving the workspace into the queued/starting state, during which the user doesn't have any feedback. To address the issue we: - stopped the main poll loop from updating the environment - moved the environment in the queued state immediately after the start button was pushed. - started a poll loop that moved the workspace from queued state to starting space only after that state became available in the backend. The intermediary stopped state is skipped by the secondary poll loop. @asher pointed out that a better approach can be implemented. We already store the status, and workspace and the agent in the environment. When the start comes in: 1. We directly update the env. status to "queued" 2. We only change the environment status if there is difference in the existing workspace&agent status vs the status from the main poll loop 3. no secondary poll loop is needed.
1 parent e24f564 commit 7c64008

File tree

1 file changed

+34
-39
lines changed

1 file changed

+34
-39
lines changed

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

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import kotlinx.coroutines.launch
3737
import kotlinx.coroutines.withTimeout
3838
import java.io.File
3939
import java.nio.file.Path
40-
import java.util.concurrent.atomic.AtomicBoolean
4140
import kotlin.time.Duration.Companion.minutes
4241
import kotlin.time.Duration.Companion.seconds
4342

@@ -55,14 +54,14 @@ class CoderRemoteEnvironment(
5554
private var workspace: Workspace,
5655
private var agent: WorkspaceAgent,
5756
) : RemoteProviderEnvironment("${workspace.name}.${agent.name}"), BeforeConnectionHook, AfterDisconnectHook {
58-
private var wsRawStatus = WorkspaceAndAgentStatus.from(workspace, agent)
57+
private var environmentStatus = WorkspaceAndAgentStatus.from(workspace, agent)
5958

6059
override var name: String = "${workspace.name}.${agent.name}"
6160
private var isConnected: MutableStateFlow<Boolean> = MutableStateFlow(false)
6261
override val connectionRequest: MutableStateFlow<Boolean> = MutableStateFlow(false)
6362

6463
override val state: MutableStateFlow<RemoteEnvironmentState> =
65-
MutableStateFlow(wsRawStatus.toRemoteEnvironmentState(context))
64+
MutableStateFlow(environmentStatus.toRemoteEnvironmentState(context))
6665
override val description: MutableStateFlow<EnvironmentDescription> =
6766
MutableStateFlow(EnvironmentDescription.General(context.i18n.pnotr(workspace.templateDisplayName)))
6867
override val additionalEnvironmentInformation: MutableMap<LocalizableString, String> = mutableMapOf()
@@ -71,7 +70,6 @@ class CoderRemoteEnvironment(
7170
private val networkMetricsMarshaller = Moshi.Builder().build().adapter(NetworkMetrics::class.java)
7271
private val proxyCommandHandle = SshCommandProcessHandle(context)
7372
private var pollJob: Job? = null
74-
private val startIsInProgress = AtomicBoolean(false)
7573

7674
init {
7775
if (context.settingsStore.shouldAutoConnect(id)) {
@@ -84,7 +82,7 @@ class CoderRemoteEnvironment(
8482

8583
private fun getAvailableActions(): List<ActionDescription> {
8684
val actions = mutableListOf<ActionDescription>()
87-
if (wsRawStatus.canStop()) {
85+
if (environmentStatus.canStop()) {
8886
actions.add(Action(context, "Open web terminal") {
8987
context.desktop.browse(client.url.withPath("/${workspace.ownerName}/$name/terminal").toString()) {
9088
context.ui.showErrorInfoPopup(it)
@@ -114,7 +112,7 @@ class CoderRemoteEnvironment(
114112
}
115113
)
116114

117-
if (wsRawStatus.canStart()) {
115+
if (environmentStatus.canStart()) {
118116
if (workspace.outdated) {
119117
actions.add(Action(context, "Update and start") {
120118
val build = client.updateWorkspace(workspace)
@@ -123,34 +121,18 @@ class CoderRemoteEnvironment(
123121
)
124122
} else {
125123
actions.add(Action(context, "Start") {
126-
try {
127-
// needed in order to make sure Queuing is not overridden by the
128-
// general polling loop with the `Stopped` state
129-
startIsInProgress.set(true)
130-
val startJob = context.cs
131-
.launch(CoroutineName("Start Workspace Action CLI Runner") + Dispatchers.IO) {
132-
cli.startWorkspace(workspace.ownerName, workspace.name)
133-
}
134-
// cli takes 15 seconds to move the workspace in queueing/starting state
135-
// while the user won't see anything happening in TBX after start is clicked
136-
// During those 15 seconds we work around by forcing a `Queuing` state
137-
while (startJob.isActive && client.workspace(workspace.id).latestBuild.status.isNotStarted()) {
138-
state.update {
139-
WorkspaceAndAgentStatus.QUEUED.toRemoteEnvironmentState(context)
140-
}
141-
delay(1.seconds)
124+
context.cs
125+
.launch(CoroutineName("Start Workspace Action CLI Runner") + Dispatchers.IO) {
126+
cli.startWorkspace(workspace.ownerName, workspace.name)
142127
}
143-
startIsInProgress.set(false)
144-
// retrieve the status again and update the status
145-
update(client.workspace(workspace.id), agent)
146-
} finally {
147-
startIsInProgress.set(false)
148-
}
149-
}
150-
)
128+
// cli takes 15 seconds to move the workspace in queueing/starting state
129+
// while the user won't see anything happening in TBX after start is clicked
130+
// During those 15 seconds we work around by forcing a `Queuing` state
131+
updateStatus(WorkspaceAndAgentStatus.QUEUED)
132+
})
151133
}
152134
}
153-
if (wsRawStatus.canStop()) {
135+
if (environmentStatus.canStop()) {
154136
if (workspace.outdated) {
155137
actions.add(Action(context, "Update and restart") {
156138
val build = client.updateWorkspace(workspace)
@@ -170,12 +152,14 @@ class CoderRemoteEnvironment(
170152
actions.add(Action(context, "Delete workspace", highlightInRed = true) {
171153
context.cs.launch(CoroutineName("Delete Workspace Action")) {
172154
var dialogText =
173-
if (wsRawStatus.canStop()) "This will close the workspace and remove all its information, including files, unsaved changes, history, and usage data."
155+
if (environmentStatus.canStop()) "This will close the workspace and remove all its information, including files, unsaved changes, history, and usage data."
174156
else "This will remove all information from the workspace, including files, unsaved changes, history, and usage data."
175157
dialogText += "\n\nType \"${workspace.name}\" below to confirm:"
176158

177159
val confirmation = context.ui.showTextInputPopup(
178-
if (wsRawStatus.canStop()) context.i18n.ptrl("Delete running workspace?") else context.i18n.ptrl("Delete workspace?"),
160+
if (environmentStatus.canStop()) context.i18n.ptrl("Delete running workspace?") else context.i18n.ptrl(
161+
"Delete workspace?"
162+
),
179163
context.i18n.pnotr(dialogText),
180164
context.i18n.ptrl("Workspace name"),
181165
TextType.General,
@@ -264,23 +248,34 @@ class CoderRemoteEnvironment(
264248
* Update the workspace/agent status to the listeners, if it has changed.
265249
*/
266250
fun update(workspace: Workspace, agent: WorkspaceAgent) {
267-
if (startIsInProgress.get()) {
268-
context.logger.info("Skipping update for $id - workspace start is in progress")
251+
if (WorkspaceAndAgentStatus.from(this.workspace, this.agent) == WorkspaceAndAgentStatus.from(
252+
workspace,
253+
agent
254+
)
255+
) {
256+
context.logger.debug("Skipping update for $id - previous and current status ")
269257
return
270258
}
271259
this.workspace = workspace
272260
this.agent = agent
273-
wsRawStatus = WorkspaceAndAgentStatus.from(workspace, agent)
261+
// workspace&agent status can be different from "environment status"
262+
// which is forced to queued state when a workspace is scheduled to start
263+
updateStatus(WorkspaceAndAgentStatus.from(workspace, agent))
274264
// we have to regenerate the action list in order to force a redraw
275265
// because the actions don't have a state flow on the enabled property
276266
actionsList.update {
277267
getAvailableActions()
278268
}
269+
}
270+
271+
private fun updateStatus(status: WorkspaceAndAgentStatus) {
272+
environmentStatus = status
279273
context.cs.launch(CoroutineName("Workspace Status Updater")) {
280274
state.update {
281-
wsRawStatus.toRemoteEnvironmentState(context)
275+
environmentStatus.toRemoteEnvironmentState(context)
282276
}
283277
}
278+
context.logger.debug("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}")
284279
}
285280

286281
/**
@@ -310,7 +305,7 @@ class CoderRemoteEnvironment(
310305
* Returns true if the SSH connection was scheduled to start, false otherwise.
311306
*/
312307
fun startSshConnection(): Boolean {
313-
if (wsRawStatus.ready() && !isConnected.value) {
308+
if (environmentStatus.ready() && !isConnected.value) {
314309
context.cs.launch(CoroutineName("SSH Connection Trigger")) {
315310
connectionRequest.update {
316311
true
@@ -336,7 +331,7 @@ class CoderRemoteEnvironment(
336331
withTimeout(5.minutes) {
337332
var workspaceStillExists = true
338333
while (context.cs.isActive && workspaceStillExists) {
339-
if (wsRawStatus == WorkspaceAndAgentStatus.DELETING || wsRawStatus == WorkspaceAndAgentStatus.DELETED) {
334+
if (environmentStatus == WorkspaceAndAgentStatus.DELETING || environmentStatus == WorkspaceAndAgentStatus.DELETED) {
340335
workspaceStillExists = false
341336
context.envPageManager.showPluginEnvironmentsPage()
342337
} else {

0 commit comments

Comments
 (0)