Re-evaluate Async Support

[NOBLES5E]

Thanks for creating the rusqlite_migration crate! It's a valuable tool for many.

The current async support in rusqlite_migration may warrant reevaluation as it may be introducing more complexity and constraints than benefits:

• The async support has led to slower release cycle (e.g. Too strict constraint on rusqlite version #108) that prevent users from upgrading to newer rusqlite versions, and this happens again and again.
• Since rusqlite is inherently synchronous, the async wrapper doesn't provide real asynchronous benefits - it's still blocking under the hood (please advise me if there's any benefit I'm not aware of)
• Users who want non-blocking migrations can easily achieve this without dedicated async support by using tokio::spawn_blocking or similar approaches in their application code
• Removing the async support would:
  • Simplify the codebase
  • Remove dependency constraints
  • Make version upgrades easier
  • Still allow users to handle migrations asynchronously in their own code if needed

I'd appreciate your thoughts on this @cljoly .

[czocher]

Hi @NOBLES5E,
This is the course of action we have in mind with @cljoly. Separating the async part into it's own crate would benefit this project but would also be a braking change.
We have it under evaluation and should probably add it to #39.

[cljoly]

The current async support in rusqlite_migration may warrant reevaluation as it may be introducing more complexity
and constraints than benefits:

Yes, thank you for opening this issue and for the detailed points. I have given some thoughts to this over the weekend.

• The async support has led to slower release cycle (e.g. #108)
that prevent users from upgrading to newer rusqlite versions, and this happens again and again.

Yes, that’s right.
I’ll point out that another option would be to express the constraint on rusqlite as either the newest version or the last one supported by tokio_rusqlite. But then we have to support two versions, potentially introducing bugs if we did not account for change in behavior between the two versions. It would certainly add to the burden of keeping that feature.

• Since rusqlite is inherently synchronous, the async wrapper doesn't provide real asynchronous benefits - it's
still blocking under the hood (please advise me if there's any benefit I'm not aware of)
• Users who want non-blocking migrations can easily achieve this without dedicated async support by using
tokio::spawn_blocking or similar approaches in their application code

I suspect some users want something clearly labelled as “compatible with tokio” and that makes the right tokio calls under the hood. I’m not aware of other benefits either.

• Removing the async support would:
• Simplify the codebase

Specifically simplify the essentially duplicated tests and wrapping method.

• Remove dependency constraints
• Make version upgrades easier
• Still allow users to handle migrations asynchronously in their own code if needed

Agreed.

This is the course of action we have in mind with @cljoly. Separating the async part into it's own crate would benefit this project but would also be a braking change.

We have indeed talked about this.

A Possible Path Forward

Breaking change

To remove the async support, we might need to bump the major version of the crate. Admittedly, that same page mentions some mitigation strategies:

Mitigation strategies:

• Clearly document your features. If there is an internal or experimental feature, mark it as such, so that users know the status of the feature.
• Leave the old feature in Cargo.toml, but otherwise remove its functionality. Document that the feature is deprecated, and remove it in a future major SemVer release.

On the fisrt point, I believe the feature is clearly documented as alpha and unstable (in the name, alpha-async-tokio-rusqlite and in the documentation), but I certainly know the library better than the average user, so I’m happy for folks to challenge that.

We could try doing the second point, i.e. removing the content of the feature and keeping it in Cargo. Specifically, we could do it in a beta version and ask for feedback from users (about keeping the feature, about whether do a major version bump and add a few more breaking changes as suggested in #39).

Providing Async Support

Now, we have at least 3 options.

1. Remove that code entirely and add documentation to guide users to use tokio::spawn_blocking. If I understand correctly, that’s what you are suggesting @NOBLES5E

• This is the option that simplifies the code the most
• It pushes some complexity to users

2. Move the feature to a separate crate. I understand that’s the option you favor @czocher, is that correct?

• It fixes the issue we have where tokio-rusqlite can be slow to update.
• Updating is relatively straightforward for users, they can simply use the new crate if they require async support
• I think it makes ongoing development harder, because then one has to keep relatively in sync two separate code bases.

3. Where we take a connection, implement the call to tokio::spawn_blocking ourselves, instead of depending on tokio-rusqlite.

• This solves our dependency problem
• Unclear if that delivers much in the way of simplifications

---

What do you all think?

[NOBLES5E]

Thank you for considering my feedback on the async support. Since rusqlite is fundamentally synchronous, the async wrapper does not provide true asynchronous benefits and merely shifts the blocking operations to separate threads (IIRC tokio-rusqlite spawns a new thread every time a connection is needed), which may not align with all users' needs.

Most use cases can achieve the desired non-blocking behavior by leveraging tokio::spawn_blocking or similar techniques in other async runtime within their own application code. And these methods exist exactly for solving this issue. This approach offers greater flexibility and keeps the crate simpler and more maintainable. Moreover, if users require a clearly labeled "compatible with tokio" solution, it sets a precedent that could lead to unnecessary async wrappers in other synchronous crates (e.g. https://github.com/blake3-team/blake3 etc.), making the ecosystem more fragmented.

[cljoly]

I’ll accept an MR that removes the async feature (or eventually do it myself), then I’ll release a 2.0 alpha version to gather feedback on this.

[NOBLES5E]

https://github.com/cljoly/rusqlite_migration/pull/228/files

Here you go

[xuxiaocheng0201]

For me, I won't use tokio_rusqlite. Instead, I use deadpool more often. This is an async connection pool. deadpool_sync provides a similar methods to tokio_rusqlite.

[cljoly]

Thanks for sharing @xuxiaocheng0201. That's another example we could add to the documentation, regardless of whether async support is removed.