Skip to content

[ws-daemon] Ensure add handler is called before update #12878

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

Merged
merged 1 commit into from
Sep 12, 2022
Merged

Conversation

Furisto
Copy link
Member

@Furisto Furisto commented Sep 12, 2022

Description

Based on https://gitpod.slack.com/archives/C033MA45L5S/p1662729393858979 the following race condition can happen:

  • New workspace is found which means it is added to the dictionary in the dispatcher
  • We wait for the container to be ready and once that has happened we set the ContainerSeen state to true
  • We call the add handler of the listeners to process the event
  • An update event arrives and it is processed before the handler for the add event is called (the condition for the processing of the update event is that the workspace is in the dictionary and that we have seen the container, which is now fullfilled)
  • 💥 The listener has not seen the workspace yet and returns an error
  • The add handler of the listener is called and the workspace is added to the listener

Instead we wait for the add handlers to be processed before we allow update events. This means we have to hold the lock slightly longer but we call the handlers in separate go routines, so the hold duration is only increased by the amount of time it takes to spin up the go routines which should be negligible.

There is still a chance that the listeners themselves have not finished the processing of the add event when the update event handlers is called (due to being called in separate go routines), but due to the locking internal to the listeners we ensure that add and update events are processed atomically, so this does not matter.

Related Issue(s)

n.a.

How to test

Not sure, would be difficult to reproduce

Release Notes

None

Werft options:

  • /werft with-preview

@Furisto Furisto added the team: workspace Issue belongs to the Workspace team label Sep 12, 2022
@Furisto Furisto requested a review from a team September 12, 2022 13:05
@roboquat roboquat merged commit 29fc780 into main Sep 12, 2022
@roboquat roboquat deleted the fo/dispatch-race branch September 12, 2022 19:19
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/S team: workspace Issue belongs to the Workspace team
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants