Optimized beta history polling by leveraging history.update_time + loading behavior bug fixes

[Nerdinacan]

What did you do?

Periodic history content polls for updates now use the history.update_time to determine whether to perform the subsequent 4 expensive SQL queries to determine if the history has updated in the region of interest.

This functionality had to be removed when row-locking problems with the trigger which keeps history.update_time current arose, but now that the trigger is back, we can take advantage of the date field to significantly reduce the number of queries required to keep the history updated.

When a query is not performed because the history.update_time indicates nothing has changed, the contents/near endpoint now returns a 204 status (no content), and no processing occurs on the client.

Some minor changes to the client-side polling processing had to be made because previously we were always receiving a result on a poll, but now it's possible to poll and not emit a response (if a 204 occurs).

Several bug fixes got squeezed into the PR as well:

1. A bug where the scroll handle of the history scroller would fail to appear was addressed. The issue was the calculation of the width of the container that holds that scroll handle, the calculated width was too narrow resulting in the scrollbar not being displayed.
2. Loading flag behavior has been solidified. Previously the loading flag would flip only when a result returned, resulting in infinite loading when there was nothing to load. Now a timeout is used to track input and output activity from the loader, and when it settles down enough we call it "done loading".
3. Similarly, a timeout was introduced to indicate that a given set of query params have no result sets resulting in a "No Results" message. In reality, there is no difference between an empty query result and a response that is still loading, because updating the cache is a constant process. For purposes of user feedback, an arbitrary timeout period had to be implemented after which we decide that a given search query has no results. This may not always be perfect if a query legitimately goes over the 2 second timeout, but the worst case scenario is a flash of a "No results" message, followed by the results as they come in.
4. Updated the poll response to include the overall history size. This value is used to determine whether or not a history is "empty" and was required to improve the behavior of the "History is empty" message. Previously polling returned no data about the history object itself, only about the contents of that history.

Why did you make this change?

(Cite Issue number OR provide rationalization of changes if no issue exists)
(If fixing a bug, please add any relevant error or traceback)

We recently put the trigger back in, so I wanted to take advantage of the improved performance.
The rest of the bug fixes probably should have gone in a more focused bug-fix PR but were noticed by the reviewers during the course of reviewing the initial polling changes.

How to test the changes?

(select the most appropriate option; if the latter, provide steps for testing below)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:

Open up the javascript console while viewing the beta history. The first query for a given set of filters should return a 200 and presumably some results. Subsequent queries may return a 204 indicating no change and no content. Other than that, the behavior of the polling should be the same as before.

License

• I agree to license these contributions under Galaxy's current license.
• I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to galaxy-committers@lists.galaxyproject.org.

[mvdbeek]

It's mostly working well, but sometime the history stays like this indefinitely:

Switching back-and-forth will let the history load.

I'll see if that's related to this PR or not.

[mvdbeek]

OK, this also seems to happen on dev, you can sort of force it by cleaning the cache and then switching histories ... it doesn't happen on every switch. But since this happens on dev as well I don't think this PR caused it.

[Nerdinacan]

OK, this also seems to happen on dev, you can sort of force it by cleaning the cache and then switching histories ... it doesn't happen on every switch. But since this happens on dev as well I don't think this PR caused it.

@mvdbeek I've noticed this. A search is happening but there are no results and the HCP thinks it is still loading. It needs to be resolved, but it's definitely a separate issue caused by the 2 disjointed cycles. I'll open yet another PR for that. I need to figure out a better way to calculate that it is "loading" perhaps with the activity() operator.

[Nerdinacan]

@mvdbeek It was only a couple of lines, I just jammed that loading adjustment in here.

[Nerdinacan]

I don't really know what's up with that package test failure. The only thing I did was import date and json libs in that contents endpoint.

https://app.circleci.com/pipelines/github/galaxyproject/galaxy/15030/workflows/b5804cd0-28a3-49dc-9e3f-9994547ad490/jobs/138350

Am I targeting the wrong branch or something? Have the requirements changed?

[mvdbeek]

That's mypy complaining about the fallback of None for an optional import, bit surprising it just now comes up. Definitely unrelated.

[mvdbeek]

Hmm, operation success, patient dead ☠️ . It's not in the infinite loading state anymore, but now it just loads the history and says it has no items, which is wrong. You can reload the page and it'll show the correct items in the history.

[Nerdinacan]

Hmm, operation success, patient dead ☠️ . It's not in the infinite loading state anymore, but now it just loads the history and says it has no items, which is wrong. You can reload the page and it'll show the correct items in the history.

Gah. This is why I was going to put it in another PR. Ok "empty" is a flag that comes from history that is never updated during polling. So now our problem is when to show the empty message.

[Nerdinacan]

The updated history size is now returned on poll results, allowing us to update the history and determine whether it is empty resulting in proper display of the "history is empty" message. An "empty" history is one with no contents as defined by the size field of the history api response.

Additionally, the cache request can now time-out and return a response which the History component interprets as "no results" meaning the current search filters have returned an empty result, but the history itself is not empty.

Previously, the cache response would just sit there awaiting a response and although it properly displayed no results for a query with no matches, there was no way to distinguish between no results and no results YET.

[Nerdinacan]

Hmmm, both of those api tests you wrote for contents/near are passing locally once I set up an ENV file to include the tools it needs to run.

@mvdbeek
Update: I rebased and they ran clean. I wonder if they are suffering from a "date-rounding conversion then comparison" issue. For example: some of the significant digits of the input date get cut off before the comparison, that kind of thing. It's not relevant for the way the client works, but that test passes dates in using a different comparator and with different serialization. Might be worth a closer look.

[Nerdinacan]

I think the failing tests are unrelated. They are about interactive environments and a citations list endpoint?

[mvdbeek]

Hmm, this is still showing an empty history for a history that has 3 datasets (one in error, 2 in pause state). Switching to the old history panel shows the datasets. They are included in the initial contents_near response. Let me know if you need more details.

[Nerdinacan]

Hmm, this is still showing an empty history for a history that has 3 datasets (one in error, 2 in pause state). Switching to the old history panel shows the datasets. They are included in the initial contents_near response. Let me know if you need more details.

I think I am going to need your specific test scenario. I'm having trouble getting what you're describing to happen locally.

[Nerdinacan]

Okay..... turns out the history.empty is not a redundant value for history.size. I had assumed we could determine whether a history was empty by looking at its size but that is not the case when datasets are in error and no size has been associated with a history even though it has errored datasets/collections associated.

I've made the client-side history object respect the history.empty flag again.

[Nerdinacan]

After some excruciating debugging, this is looking a lot better.

There are some things I would like to improve later but they mostly will require API changes:

1. We are still running all the legacy history change handlers because of the adapters (components/History/adapters) that signal for Backbone to change history whenever Vuex does. This will disappear when we stop attempting to sync backbone to vuex, but for now we're doing roughly 2x the necessary work whenever we switch histories. My attempts to hand the history object data directly to backbone and bypass the numerous pointless ajax lookups was resulting in more bugs than improvements.
2. The API response for a history summary does not include the "empty" field which would be handy. I have to load a separate detailed version that is not the same as the summary. It would be nice to know which fields we can safely add to the summary and which are problematic for load times.
3. The API response for changing a history does not include the same fields which we retrieve when we look up the history normally, and there does not appear to be a way to modify the return fields on the "/history/create_new_current" or "/history/set_as_current" routes. As a result, whenever we change history we end up doing 2 calls instead of one to get the extra fields that would be available if the API were consistent.
4. The Vuex history store code is overly-complicated because we have to dance around the server's whims when it changes the current history.

If we're stuck with server control of the current history, I think a rewrite for a consistent and reliable switch history endpoint would be a good goal, one that has the same field configurability as the normal history lookups in /api/histories

Failing integration test looks non-relevant.

[mvdbeek]

This is working really well now, thanks a lot @Nerdinacan!