Skip to content
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

Cancel doesnt actually cancel a query in redshift. #598

Closed
podarsmarty opened this issue Oct 9, 2015 · 9 comments
Closed

Cancel doesnt actually cancel a query in redshift. #598

podarsmarty opened this issue Oct 9, 2015 · 9 comments

Comments

@podarsmarty
Copy link

I was using redash and wrote a pretty bad query. Soon after I hit cancel and proceeded to run other queries but they wouldn't go through. I suspected redshift to be busy running my previous erroneous query so logged into the aws console and sure enough it was running. I had to terminate the query manually there.

@arikfr
Copy link
Member

arikfr commented Oct 10, 2015

Cancel is implemented by terminating the process that is running the query. I've noticed that it sometimes takes time for Celery (the background workers framework) to process the cancel request. How much time did it pass between when you cancelled in re:dash and when you checked the console?

@podarsmarty
Copy link
Author

Hmm it had to be around 10 to 15 min since I let it run for a while as I worked on other things.

@alexdebrie
Copy link
Contributor

We've had a similar issue with Redshift while using redash. I think the problem is that terminating the process doesn't actually kill the query in Redshift. You need to send a cancel request to Redshift by sending the INT signal to the process.

To test this, I fired off a query that I knew would take a long time in psql. In another window, I killed it with TERM. When I checked running queries, that query was still running, even though the psql process had been killed. I then tried the same thing but sent an INT signal. This caused a cancel request to be sent and the query was cancelled.

The easy way to do this would be to add signal=INT to the revoke method. However, I don't know enough about how that interacts with non-Redshift tasks. I'm happy to help implement this if needed.

@arikfr
Copy link
Member

arikfr commented Oct 20, 2015

@alexdebrie is correct, the Postgres/Redshift adapter sends cancel request when it receives the INT signal. I was under the impression that when revoking a task it will send INT signal, but from looking at Celery documentation it looks like the default is indeed TERM.

I'll double check this again tomorrow, and fix (and test with other adapters) accordingly.

Thanks for the debugging, @alexdebrie !

@alexdebrie
Copy link
Contributor

No problem, thanks for the awesome project!

@Sigma-anthony
Copy link

No, thank You!

Regards,
Anthony

On 20 October 2015 at 22:30, Alex DeBrie notifications@github.com wrote:

No problem, thanks for the awesome project!


Reply to this email directly or view it on GitHub
#598 (comment)
.

Anthony Kalinde
Big Data Engineer
DharmicData.com

skype: anthonykalinde

mobile: +47 90 22 5857

linkedin: no.linkedin.com/in/anthonykalinde

@alexdebrie
Copy link
Contributor

@arikfr -- Just checking to see if you've had a chance to test this with other adapters. We like to stay in sync with official redash repo as much as possible, but we'll just make this fix in our repo if changing it will break other runners.

@arikfr
Copy link
Member

arikfr commented Nov 16, 2015

Started testing it now, and even when I use the INT signal (self._async_result.revoke(terminate=True, signal="SIGINT"), I see that it just kills the process but doesn't call my code that tries to catch the INT signal.

Did you have the chance to try it out?

@alexdebrie
Copy link
Contributor

Ahh darn. I haven't tried it yet. I should have some time later this week, and I'll let you know what I find.

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

4 participants