[sqllab] Help sqllab forget query history

[timifasubaa]

In the sqllab asynchronous polling logic, it looks through the query history and continues to poll if there is a query in the history that is not successful.

This PR makes it look only at the last 6 hours of queries when checking for their states. The rationale is that a query that was started 6 hours ago is probably dead or problematic and should be rerun.

@graceguo-supercat @mistercrunch @john-bodley

[mistercrunch]

I've been meaning to do something like this. I wonder if the state should be altered though, like setting the state to failed or timeout. Also build is failing...

[john-bodley]

I agree with @mistercrunch it would be great to change the state in the queries table for clarity and auditing purposes. Note this would actually be a backend change as opposed to a frontend change.

[timifasubaa]

Okay, I will change the logic to set all pending or running queries older than 6 hours old to failed.

[codecov-io]

Codecov Report

Merging #4833 into master will decrease coverage by 0.01%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #4833      +/-   ##
==========================================
- Coverage   77.09%   77.07%   -0.02%     
==========================================
  Files          44       44              
  Lines        8551     8558       +7     
==========================================
+ Hits         6592     6596       +4     
- Misses       1959     1962       +3

Impacted Files	Coverage Δ
superset/db_engine_specs.py	52.47% <ø> (ø)	⬆️
superset/views/core.py	74.62% <85.71%> (-0.1%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8933365...ab958c6. Read the comment docs.

[john-bodley]

@timifasubaa I wonder if timeout or maybe timedout(past tense) is more descriptive that failed since the later can mean multiple things.

[timifasubaa]

There is a lot of repeated logic with stop_query for the new timedout query logic.

[graceguo-supercat]

This logic should not be in front-end (browser-side).
Query table knows each query started time. it doesn't need front-end sends ajax call to set query state.
when front-end ask a query's status, server side should first check the query's start time, if

• longer than 6 hours old: set it timeout and return timeout state.
• < 6 hours: return its current status.

[timifasubaa]

I changed the logic to update the db's sate to timeout before it responds to polls.

[timifasubaa]

PING

Quick update on the state of the logic.
What is really happening is that the frontend ceases to poll for queries older than 6 hours old.
This is because on polls, it checks for all queries by that user whose state has changed after the last updated time. Meaning if a query is on pending for a very long time (> 6hours) and another query that's running at the same time changes state. We will no longer poll and also no longer receive any more updates.

Is this backend code worth having? Especially since it may trick people into believing we actually timeout the queries when the query will most likely not be updated to timeout.

[graceguo-supercat]

current auto query refresh component work like this:

• in client-side, from all queries in local storage, it find out

  • status in 'running', 'started', 'pending', 'fetching', and
  • started in < 6 hours

• if has any queries, polling with latest updated timestamp

• if there is no such queries, stop polling.

• in server-side, for every poll request, we get all queries

  • by user_id, and
  • changed after updated timestamp

• if some query went wrong, and query status kept as running but never got to updated after 6 hours, when client-side polling with a newer timestamp (< 6 hours), this query will not have a chance to be set as timeout state. But it won't cause client-side keep polling because now client-side added timeout threshold.

this PR LGTM. but will not merge until @mistercrunch feel comfortable. Thank you!

[timifasubaa]

PING @mistercrunch @john-bodley

[mistercrunch]

No hard coded values please. Put it in src/SqlLab/constants.js

[timifasubaa]

Done

[mistercrunch]

Any specific reason to not follow the default "Squash and merge" ?