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

Query cancellation/timeout support #123

Open
penberg opened this issue Jun 27, 2024 · 7 comments
Open

Query cancellation/timeout support #123

penberg opened this issue Jun 27, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@penberg
Copy link
Contributor

penberg commented Jun 27, 2024

If a query takes a long time, applications may want to cancel them either manually or via a timeout mechanism. We should be able to use the sqlite3_progress_handler() to implement this where SQLite periodically checks if it's OK to keep running. We would have to track the execution time of individual queries.

Depends on #122 for infrastructure on query timing.

@penberg penberg added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Jun 27, 2024
penberg added a commit that referenced this issue Dec 17, 2024
@haaawk
Copy link
Contributor

haaawk commented Dec 18, 2024

Is this fixed @penberg or is there anything left for @levydsa to do?

@penberg
Copy link
Contributor Author

penberg commented Dec 18, 2024

@haaawk I only implemented brute force canceling all ongoing operations. We still need to do timeouts, for example.

@haaawk
Copy link
Contributor

haaawk commented Dec 18, 2024

How are we going to allow cancelation of a single queries @penberg? by making sure there's at most a single operation at each connection so that we can be sure that when we interrupt that connection we cancel a particular query?

@haaawk
Copy link
Contributor

haaawk commented Dec 18, 2024

Here's the idea @levydsa @LucioFranco and me have about doing this (based on https://github.com/tursodatabase/libsql-js/blob/main/src/database.rs):

  1. We add running_queries_count and running_cancellable_query to Database possibly atomics
  2. Every time js_exec_sync or js_exec_async run they increase running_queries_count and every time they finish they decrease it
  3. Both js_exec_sync or js_exec_async check that running_cancellable_query is false and if not they error out saying that there can't be any other query while a query with timeout is running
  4. We add js_exec_sync_with_timeout and js_exec_async_with_timeout which set running_cancellable_query to true when starting and to false after they finish
  5. Both new methods check that running_cancellable_query is false and that running_queries_count is 0 or they error out saying that query with timeout can't run in parallel to other queries
  6. Both new methods spawn a timer that if run calls interrupt on the connection and sets running_cancellable_query to false. We need to make sure that the timer is cancelled when the query finishes successfully.

This way we make sure there's at most one cancellable query on the connection and we can safely interrupt it.

@penberg
Copy link
Contributor Author

penberg commented Dec 18, 2024

@haaawk I am not sure I understand the proposal. How will you interrupt a query that is running within the SQLite VDBE? See the Database.interrupt() test case in integration-tests/tests/async.test.js, for example, which is essentially an infinite loop within VDBE.

@penberg
Copy link
Contributor Author

penberg commented Dec 18, 2024

I still think (ab)using the progress handler is better here and just document the limitation that it only works for a single query at a time.

@haaawk
Copy link
Contributor

haaawk commented Dec 18, 2024

@haaawk I am not sure I understand the proposal. How will you interrupt a query that is running within the SQLite VDBE? See the Database.interrupt() test case in integration-tests/tests/async.test.js, for example, which is essentially an infinite loop within VDBE.

I would implement it in https://github.com/tursodatabase/libsql-js/blob/main/src/database.rs in js_exec_async_with_timeout by spawning a timeout that calls conn.interrupt. Wouldn't it work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants