Skip to content
New issue

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

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

Already on GitHub? # to your account

Can't cancel long-running queries #514

Closed
krlmlr opened this issue Oct 19, 2024 · 2 comments · Fixed by #515
Closed

Can't cancel long-running queries #514

krlmlr opened this issue Oct 19, 2024 · 2 comments · Fixed by #515
Labels
feature a feature request or enhancement

Comments

@krlmlr
Copy link
Collaborator

krlmlr commented Oct 19, 2024

Not in the RStudio IDE, not in the plain terminal. Hitting Ctrl + C when running this script will achieve nothing.

@hannes: I checked, we either need to call R_CheckUserInterrupt() periodically, or perhaps install our own signal handler and remove it for every single call to an R API. Is there a callback from duckdb that would be fired regularly from the main thread?

library(DBI)
con <- dbConnect(duckdb::duckdb())

dbExecute(con, "CREATE TABLE data AS SELECT unnest(generate_series(1, 100000)) AS a")
#> [1] 1e+05

system.time(dbGetQuery(con, "SELECT COUNT(*) FROM data JOIN data AS data2 ON data.a != data2.a"))
#>    user  system elapsed 
#>  10.027   0.060  10.514

dbDisconnect(con)

Created on 2024-10-19 with reprex v2.1.1

@krlmlr
Copy link
Collaborator Author

krlmlr commented Oct 20, 2024

We already use PendingQuery() and then ExecuteTask(), but for this particular query, cancellation works only "sometimes". Need to review timing of the checks, perhaps there's a long non-interruptible call to ExecuteTask() .

Draft branch: https://github.com/duckdb/duckdb-r/tree/f-514-cancel . Looks like we need to check R's internal R_interrupts_pending, seen this in other packages too.

Example that works flawlessly in the terminal and in RStudio: https://github.com/krlmlr/cancel.test .

Assuming that PendingQuery() can also be used for relational objects. We'll see.

@krlmlr
Copy link
Collaborator Author

krlmlr commented Oct 20, 2024

The query is interesting: sometimes it issues only a handful of tasks, one of them very long-running, sometimes a lot of tiny tasks. This explains the "sometimes".

The PR contains a draft that should work on non-Windows. We no longer need PendingQuery(), instead it installs its own temporary signal handler, which is safe at this stage.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant