tikv / fail-rs

Fail points for rust
Apache License 2.0
338 stars 40 forks source link

support async failpoints #73

Open tabokie opened 12 months ago

tabokie commented 12 months ago

Signed-off-by: Xinye xinye.tao@metabit-trading.com

BusyJay commented 12 months ago

Seems duplicated with #58.

tabokie commented 12 months ago

Seems duplicated with #58.

From the looks of it, #58 doesn't support async sleep and runtime async callback. This PR supports them, but doesn't support specifying a compile-time async callback (the one implemented in-place with fail_point).

I can't figure out an elegant way to unify async and sync version of fail_point!(name, callback). And I'm reluctant to go with #58 and make all fail_point!(name, callback) return async blocks, because most of the time that isn't necessary. If the user want to inject custom async logic, they can always use the more powerful cfg_callback.

waynexia commented 12 months ago

I can't figure out an elegant way to unify async and sync version of fail_point!(name, callback). And I'm reluctant to go with https://github.com/tikv/fail-rs/pull/58 and make all fail_point!(name, callback) return async blocks, because most of the time that isn't necessary. If the user want to inject custom async logic, they can always use the more powerful cfg_callback.

FYI, #58 defines two macros for sync (fail_point!()) and async(async_fail_point!()). It won't affect existing code.

tabokie commented 12 months ago

I can't figure out an elegant way to unify async and sync version of fail_point!(name, callback). And I'm reluctant to go with #58 and make all fail_point!(name, callback) return async blocks, because most of the time that isn't necessary. If the user want to inject custom async logic, they can always use the more powerful cfg_callback.

FYI, #58 defines two macros for sync (fail_point!()) and async(async_fail_point!()). It won't affect existing code.

I see, it is a slightly different mental model. Your idea is to only use async_fail_point when user want to inject async code. My idea is to always use async_fail_point in async context. In practice the latter is more versatile because we might not know what to inject beforehand.

And continue with my idea, it becomes important to allow user to write async_fail_point(name, || { Ok(()) }) instead of async_fail_point(name, || { async { Ok(()) } }.