Skip to content

393 fix cached dataset load#406

Open
jvigliotta wants to merge 5 commits intomainfrom
realtime-session-availability
Open

393 fix cached dataset load#406
jvigliotta wants to merge 5 commits intomainfrom
realtime-session-availability

Conversation

@jvigliotta
Copy link
Copy Markdown
Collaborator

Implements #393

Fixes an issue in getTopicsWithSessions interaction with persistence storage that caused a minimum of 15s delay before realtime sessions were stored.

old behavior: getTopicsWithSessions would check the dataset cache with getDatasets. The problem is that this is a race condition with MIO loads, as when it runs there are almost never any datasets loaded yet.

new behavior:

  1. Added an async function that will check the getDatasets cache every 1 second for up to 15 seconds before returning an empty set. If a new dataset is found during this iteraction, will check for another dataset 1s later in case MIOS are still loading.
  2. Added a new flag to getTopicsWithSessions which will toggle between the new behavior and the old behavior. This preserves the instant response on user interaction with the realtime session selection UI.

@jvigliotta
Copy link
Copy Markdown
Collaborator Author

Replaces: #395

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Comment thread src/services/session/SessionService.js Outdated
Comment thread src/services/session/SessionService.js Outdated
Comment thread src/services/session/SessionService.js Outdated
Comment thread src/services/session/SessionService.js
Copy link
Copy Markdown
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good stuff. I wonder, if we are going for initial load time here, I think we can explicitly wait for all datasets to load intially, then start polling afterwards to monitor for changes.

@jvigliotta
Copy link
Copy Markdown
Collaborator Author

This is a good stuff. I wonder, if we are going for initial load time here, I think we can explicitly wait for all datasets to load intially, then start polling afterwards to monitor for changes.

I don't know that we can, wouldn't we need to know how many datasets exist to do this? I think this is a good solution for the "let's be able to select a session as soon as we can" situation. Even if on the off chance a dataset comes in after the 1 second wait time in SessionService, they'd still see that the next time poll runs 15 seconds later in RealtimeSessionIndictator, so I think this is a pretty solid solution.

…had thrown, using strict equality, some code cleanup and logic fixes to prevent loops
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants