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

Internal use of async #8

Open
AntonyCorbett opened this issue Mar 23, 2022 · 5 comments
Open

Internal use of async #8

AntonyCorbett opened this issue Mar 23, 2022 · 5 comments

Comments

@AntonyCorbett
Copy link
Contributor

What are your thoughts on async in Microsoft.Data.Sqlite.Core? I think the following advice is still relevant:

https://docs.microsoft.com/en-us/dotnet/standard/data/sqlite/async

It would be difficult to remove the public Async method signatures on SqliteCache, but perhaps the underlying database operations could be changed to use sync methods and so minimize the async overhead. It's only a very minor consideration.

@mqudsi
Copy link
Member

mqudsi commented Apr 6, 2022

Thanks for opening this issue. I think this is a fair observation and certainly something doable. We can't remove the async wrapper methods even if we wanted to as they're required by the IDistributedCache interface we implement, but we can at least minimize the impact internally.

One question is how the sync api (in practice) handles cancellation as compared to the async api. Does the cancellation token for the async SQLite API basically do nothing?

@AntonyCorbett
Copy link
Contributor Author

One question is how the sync api (in practice) handles cancellation as compared to the async api. Does the cancellation token for the async SQLite API basically do nothing?

Yes, it looks like we'd get Task.FromCanceled<> if the token cancellation is already requested on entry to the API, But subsequent cancellation is ignored.

@mqudsi
Copy link
Member

mqudsi commented Apr 11, 2022

The real question is if there are any cases (not just the straightforward one) where subsequent cancellation works in the asynchronous API (since we already know that the async api is just a wrapper around the synchronous one) that don't work in the synchronous one.

As an aside, I'm very surprised that subsequent cancellation in the async API is completely ignored since sqlite3_exec takes a callback parameter and any sane application using the sqlite3 C/FFI API will avail itself of that to offer an option to abort long-running queries. I haven't dug into the Microsoft.Data.Sqlite.Core code, but I imagine it installs such a callback?

@AntonyCorbett
Copy link
Contributor Author

I'm very surprised that subsequent cancellation in the async API is completely ignored

You're right - CancellationTokenRegistration is used to register a callback that cancels the execution.

@AntonyCorbett
Copy link
Contributor Author

The observation that the cancellation token can be used to abort long-running statements makes the MS advice to avoid async Sqlite calls a little simplistic.

@mqudsi mqudsi changed the title Async Internal use of async Jun 29, 2023
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

2 participants