flatironinstitute / mcmc-monitor

Monitor MCMC runs in the browser
Other
35 stars 0 forks source link

Terminate sequence-data polling when there is no longer an active chain #17

Closed jsoules closed 1 year ago

jsoules commented 1 year ago

While working on issue #8, I've noticed that polling for expired sequence-statistics and variable-statistics data continues, even after navigating away from the actual data set. This isn't a big deal with data sizes we have (assuming we aren't needlessly recomputing), but it would be better to avoid the gap.

The right approach here is probably to put something in place to (at least) call the MCMCDataManager.stop() function as a hook cleanup for the main page, so it'll get called by React when the user navigates away.

jsoules commented 1 year ago

So it looks like this polling is activated in App.tsx when the SetupMCMCMonitor component is used to wrap the main page. The MCMCDataManager gets instantiated in a useEffect hook in SetupMCMCMonitor. This is meant to keep the manager as a singleton, and re-fires only on change to the dispatch method exposed by the MCMCMonitorReducer (so, effectively never, because we never really want to clear out this cache).

This hook does have a cleanup method set to call stop() on the data manager. However, as I read this, what's probably happening here is that because the hook fires outside the MainWindow component, rather than one of the individual pages, it does not actually get a close event when the user navigates away from the RunPage component that has to be active for the polling to make sense. (When RunPage is not being displayed, there can be no meaningful selected run ID, let alone chains/sequences attached to it. So we're polling for data that can't be displayed.)

I think instantiating the MCMCDataManager in SetupMCMCMonitor.tsx and keeping that cache around for the lifetime of the application is probably the right thing to do. However, I think calling its start() and stop() functions might be better served by a separate useEffect hook in the RunPage object, similar to what's done for the update refresh calls (via useMCMCMonitor's`updateKnownData(runId) call).

Will look into this more.