[sqllab] Help sqllab forget query history #4833

timifasubaa · 2018-04-16T21:50:46Z

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 · 2018-04-18T04:04:25Z

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 · 2018-04-18T17:56:18Z

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 · 2018-04-18T21:18:55Z

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

codecov-io · 2018-04-18T21:26:06Z

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 · 2018-04-18T22:03:32Z

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

timifasubaa · 2018-04-20T18:54:51Z

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

graceguo-supercat · 2018-04-22T20:24:24Z

superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx

@@ -16,12 +17,23 @@ class QueryAutoRefresh extends React.PureComponent {
  componentWillUnmount() {
    this.stopTimer();
  }
+  timeoutOldQueries() {


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 · 2018-04-24T01:10:39Z

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

timifasubaa · 2018-04-27T23:54:08Z

@john-bodley @mistercrunch @graceguo-supercat ready for review

timifasubaa · 2018-05-01T19:12:01Z

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 · 2018-05-01T20:39:33Z

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 · 2018-05-04T04:01:50Z

PING @mistercrunch @john-bodley

mistercrunch · 2018-05-07T16:51:06Z

superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx

+    return Object.values(queries)
+      .some(
+        q => ['running', 'started', 'pending', 'fetching'].indexOf(q.state) >= 0 &&
+        now - q.startDttm < 21600000,


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

mistercrunch · 2018-05-08T06:20:30Z

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

…e_past [sqllab] Help sqllab forget query history

(cherry picked from commit 2014329)

(cherry picked from commit 03cae41)

(cherry picked from commit 2014329)

timifasubaa force-pushed the help_sqllab_forget_the_past branch 6 times, most recently from 163ddda to 5ea9dd9 Compare April 16, 2018 22:30

help sqllab forget the past

e47d8a5

timifasubaa force-pushed the help_sqllab_forget_the_past branch from 5ea9dd9 to e47d8a5 Compare April 18, 2018 21:09

timifasubaa force-pushed the help_sqllab_forget_the_past branch 6 times, most recently from 138dc75 to ab18e5f Compare April 20, 2018 22:20

graceguo-supercat reviewed Apr 22, 2018

View reviewed changes

timifasubaa force-pushed the help_sqllab_forget_the_past branch 8 times, most recently from 1ed8142 to 9d2e516 Compare April 24, 2018 01:09

timifasubaa force-pushed the help_sqllab_forget_the_past branch from b922df7 to a43bc2f Compare April 27, 2018 23:53

timifasubaa force-pushed the help_sqllab_forget_the_past branch 5 times, most recently from dc1ef37 to a67faa0 Compare April 29, 2018 00:24

mistercrunch requested changes May 7, 2018

View reviewed changes

make queries older than 6 hours timeout

ab958c6

timifasubaa force-pushed the help_sqllab_forget_the_past branch from a67faa0 to ab958c6 Compare May 7, 2018 17:14

mistercrunch approved these changes May 7, 2018

View reviewed changes

timifasubaa merged commit d87504c into apache:master May 7, 2018

wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018

Merge pull request apache#4833 from timifasubaa/help_sqllab_forget_th…

37998c3

…e_past [sqllab] Help sqllab forget query history

graceguo-supercat mentioned this pull request May 2, 2019

[sql lab] Fix setting async query status to timed_out #7429

Closed

12 tasks

graceguo-supercat mentioned this pull request May 13, 2019

Reduce db queries in /queries/ endpoint #7498

Merged

12 tasks

graceguo-supercat pushed a commit to graceguo-supercat/superset that referenced this pull request May 24, 2019

[sql lab]revert apache#4833

70c0868

graceguo-supercat pushed a commit that referenced this pull request May 24, 2019

[sql lab]revert #4833 (#7498)

2014329

john-bodley pushed a commit that referenced this pull request May 24, 2019

[sql lab]revert #4833 (#7498)

3b01f4e

(cherry picked from commit 2014329)

michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 31, 2019

[sql lab]revert apache#4833 (apache#7498)

19f33c3

(cherry picked from commit 2014329)

michellethomas pushed a commit that referenced this pull request Jun 1, 2019

[sql lab]revert #4833 (#7498)

1e7f30f

(cherry picked from commit 2014329)

betodealmeida mentioned this pull request Oct 27, 2020

fix: prevent race condition when canceling query #11449

Merged

6 tasks

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024

[sql lab]revert apache#4833 (apache#7498)

70afe9e

(cherry picked from commit 03cae41)

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024

[sql lab]revert apache#4833 (apache#7498)

cd2375a

(cherry picked from commit 2014329)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sqllab] Help sqllab forget query history #4833

[sqllab] Help sqllab forget query history #4833

timifasubaa commented Apr 16, 2018 •

edited

Loading

mistercrunch commented Apr 18, 2018

john-bodley commented Apr 18, 2018

timifasubaa commented Apr 18, 2018 •

edited

Loading

codecov-io commented Apr 18, 2018 •

edited

Loading

john-bodley commented Apr 18, 2018 •

edited

Loading

timifasubaa commented Apr 20, 2018

graceguo-supercat Apr 22, 2018

timifasubaa commented Apr 24, 2018

timifasubaa commented Apr 27, 2018

timifasubaa commented May 1, 2018

graceguo-supercat commented May 1, 2018

timifasubaa commented May 4, 2018

mistercrunch May 7, 2018

timifasubaa May 7, 2018

mistercrunch commented May 8, 2018

[sqllab] Help sqllab forget query history #4833

[sqllab] Help sqllab forget query history #4833

Conversation

timifasubaa commented Apr 16, 2018 • edited Loading

mistercrunch commented Apr 18, 2018

john-bodley commented Apr 18, 2018

timifasubaa commented Apr 18, 2018 • edited Loading

codecov-io commented Apr 18, 2018 • edited Loading

Codecov Report

john-bodley commented Apr 18, 2018 • edited Loading

timifasubaa commented Apr 20, 2018

graceguo-supercat Apr 22, 2018

Choose a reason for hiding this comment

timifasubaa commented Apr 24, 2018

timifasubaa commented Apr 27, 2018

timifasubaa commented May 1, 2018

graceguo-supercat commented May 1, 2018

timifasubaa commented May 4, 2018

mistercrunch May 7, 2018

Choose a reason for hiding this comment

timifasubaa May 7, 2018

Choose a reason for hiding this comment

mistercrunch commented May 8, 2018

timifasubaa commented Apr 16, 2018 •

edited

Loading

timifasubaa commented Apr 18, 2018 •

edited

Loading

codecov-io commented Apr 18, 2018 •

edited

Loading

john-bodley commented Apr 18, 2018 •

edited

Loading