Prevent UI lockup caused by PubSub race condition #9
Conversation
This file contains 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
|
Damn, I've managed to still encounter the error 😞 |
Should we just change the front end to re-request the current id when this failure occurs? I can have a look at this after the festivities. |
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.
When running tests multiple times, occasionally we see an error "HTTP request for phase descriptors failed." in the web UI when a test completes, preventing us from starting the test again.
From some sleuthing in the web ui console, I noticed that the PubSub websocket publishes (what it believes to be) the test UID of the currently running test. The web ui then requests
tests/<test_uid>/phases, which should return the phase info for that test. However, when we see this error, I noticed that it's sending the request of the previous test. The python only stores the information of the currently running test, and so the request for the previous test info fails.The issue here (I believe) is that when a test is finishing and a new test is started, the event for the previous test finishing arrives after the notification for the new test starting. This means we store the UID of the previous test in
StationPubSub, and incorrectly report that test UID to the webui, causing it to request the phase info for the old test.My hacky workaround for this is to only allow changing the UID for the test when we've never seen the test UID before. If we see an update for an old test, we refuse to update the uid.