[GitHub] [incubator-superset] graceguo-supercat opened a new pull request #7523: [sql lab] Fix new query stuck at pending state

[GitBox <gi...@apache.org>]

graceguo-supercat opened a new pull request #7523: [sql lab] Fix new query stuck at pending state
URL: https://github.com/apache/incubator-superset/pull/7523
 
 
   ### CATEGORY
   
   Choose one
   
   - [x] Bug Fix
   - [ ] Enhancement (new features, refinement)
   - [ ] Refactor
   - [ ] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   I found a very strange corner case: some user didn't use sql lab for a long time, and they didn't have any old query tab in Sql Lab. When recently they started to use sql lab, their async query may stuck at pending state forever. From browser console I can see the query was sent to back end, but browser didn't start polling query results at all. So that even backend finished query, front end is still showing pending state. This issue can't be consistently reproduced, it only happened on few users, but those users are totally blocked and can't run any async queries successfully.
   
   After debug I found polling was stopped by this line:
   
   > return (
         queriesLastUpdate > 0 &&
         Object.values(queries).some(
           q => isQueryRunning(q) &&
           now - q.startDttm < MAX_QUERY_AGE_TO_POLL,
         )
       );
   For those users got stuck at pending state, when sql lab was initialized, `queriesLastUpdate` was 0. There was a change for initial value introduced in #5896. `queriesLastUpdate` was set to 0. So i am thinking probably those users' sql lab state was persisted into localStorage. And now they open sql lab with new code base that requires `queriesLastUpdate > 0`, then their sql lab can't poll anymore.
   
   I don't think  `queriesLastUpdate > 0` is really necessary for polling query results. So my solution for this issue is just remove this requirement.
   
   ### TEST PLAN
   ci and check with users stuck at pending state. 
   
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   ### REVIEWERS
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org