Skip to content

Stopping queries running with Gitbase doesn't work #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
se7entyse7en opened this issue May 9, 2019 · 4 comments
Closed

Stopping queries running with Gitbase doesn't work #35

se7entyse7en opened this issue May 9, 2019 · 4 comments

Comments

@se7entyse7en
Copy link
Contributor

No description provided.

@se7entyse7en se7entyse7en changed the title Killing queries running with Gitbase doesn't work Stopping queries running with Gitbase doesn't work May 9, 2019
@carlosms
Copy link
Contributor

More context: queries seem to be cancelled correctly from superset point of view, there are no errors reported to the user. But the query will continue running in gitbase, which means superset cancels by forgetting about the results, but not calling KILL.

@smacker
Copy link
Contributor

smacker commented May 17, 2019

Superset doesn't really stop query.
The source code of the stop endpoint: https://github.com/src-d/superset-compose/blob/master/superset/superset/views/core.py#L2456
It only updates Query model instance in database changing the status.

The model: https://github.com/src-d/superset-compose/blob/master/superset/superset/models/sql_lab.py#L36
It doesn't have any hook that would cancel underlying queries.

Running query calls db_engine_spec.execute at the end: https://github.com/src-d/superset-compose/blob/master/superset/superset/sql_lab.py#L191

There is no hook to check status and cancel the query on EngineSpec level. handle_cursor is called only when the query is finished.

I'm currently exploring how difficult it is to add query cancelation support.

@smacker
Copy link
Contributor

smacker commented May 17, 2019

I implemented cancellation: https://github.com/src-d/superset-compose/pull/53/files
But it doesn't work due to a bug in gitbase: src-d/gitbase#817

The PR also doesn't have tests. Most probably after tests are written it makes sense to rebase on superset master and suggest the change to upstream.

@smacker
Copy link
Contributor

smacker commented May 30, 2019

fixed

@smacker smacker closed this as completed May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants