Support asio::cancel_after

[anarthal]

This is a very handy token, but it requires the initiation function object to have a bound executor. The following should work:

redis::connection conn (ctx);
co_await conn.async_exec(req, res, asio::cancel_after(10s));

[mzimbres]

I tried to implement cancelation for async_exec here:

redis/include/boost/redis/detail/connection_base.hpp

Line 149 in d910557

if (is_cancelled(self)) {

At the moment I am considering removing as it is hard and I am not sure there is a good use cases for it. Do you think this is worth doing?

[anarthal]

Yes, there are good reasons. Without cancellation, you can't run a request with a timeout, for instance.

[mzimbres]

I am trying to understand how async_exec cancellation should work. There are four states to consider

1. waiting: Request wasn't written to the socket yet.
2. staged: Request is being written to the socket i.e. async_write is suspended.
3. written: The payload was written to the socket i.e. async_write completed with success. At this point the request is either waiting for a response if one is expected or done if none is expected.
4. done: Response arrived and completion is queued for completion.

Taking that into account

• total: cancellation is only possible at 1.
• partial: can not be honored or is too difficult to implement. Should I ignore this type of cancellation (except when in 1.)?
• terminal: Can be honored most of the time except at 4. where the request was executed with success, in that case cancellation must be ignored?

@anarthal: How does this work in Boost.Mysql?

Some things are still not clear to me, will cancellation cause the operation to complete with an error code e.g. operation_cancelled or does it complete with success and I have to look at the cancellation state, for example, with async_compose I do

self.get_cancellation_state().cancelled() != asio::cancellation_type_t::none;

@klemens-morgenstern, Please, chime in if you think I am misunderstanding cancellation. thanks.

[klemens-morgenstern]

You are correct. At 4 you should just ignore all cancellation requests.

[anarthal]

There's actually one more hidden case:

waiting: Request wasn't written to the socket yet.
staged: Request is being written to the socket i.e. async_write is suspended.
just after staged: request has been written successfully, but its handler hasn't run yet. If we are to wait for a response, async_wait hasn't been called yet.
written: The payload was written to the socket i.e. async_write completed with success. At this point the request is either waiting for a response if one is expected or done if none is expected.
done: Response arrived and completion is queued for completion.

Detection goes like this:

waiting: you need to check self.get_cancellation_state()
staged: async_write will fail with asio::error::operation_aborted
just after staged: you need to check self.get_cancellation_state()
written: if you're waiting a response, the wait operation will finish with asio::error::operation_aborted. If you're using a timer, you might need to check self.get_cancellation_state() to disambiguate, since cancelling a timer causes the same error code.
done: as Klemens said, you should just complete the operation here. Nothing to check.

asio::cancel_after does terminal cancellation by default, so I'm inclined to think that's the one that is more important to support. I'd support terminal as you described, and maybe total/partial for requests that haven't been queued yet, as you proposed.

In Boost.MySQL:

• For any_connection (it does no queuing), only terminal is supported. It is implemented as a sans-io state machine that outputs what to do next, plus an async_compose that just does what the state machine says (https://github.com/boostorg/mysql/blob/90405e79e4590f142e28094eea1625f259ffeec4/include/boost/mysql/detail/engine_impl.hpp#L46). The async_compose loop checks self.get_cancellation_state().
• connection_pool supports all cancellation types because async_get_connection can be cancelled without side effects.

I believe we could extend what you did in the multiplexer (great job BTW) to take most of the logic in async_exec outside of async_compose. Would you like me to experiment with that next week?

[mzimbres]

I believe we could extend what you did in the multiplexer (great job BTW) to take most of the logic in async_exec outside of async_compose. Would you like me to experiment with that next week?

Yes, go ahead, thanks. That is actually very important otherwise testing is very hard and limited. Notice that support for cancellation in async_exec will eliminate the need for cancel_if_not_connected, which will come as breaking change.

[anarthal]

Why is that? It looks like cancel_if_not_connected serves a different purpose than regular cancellation, right? cancel_if_not_connected is like "do you want to cancel your request or wait for a connection to be established"? While regular cancellation is a tool to say "hey cancel this request, regardless of what the connection is doing". What do you think?

[criatura2]

The problem is that cancel_if_not_connected does not do what it claims. Being connected should mean a command can be executed but all the implementation does is to check whether the socket is open, which is not strong enough.

If cancellation is supported users like this won't need to know whether the connection stands since they can simply send the request with a timeout. If the timer fires it is not connected (or the timeout was too short).

[anarthal]

I see what you mean. I plan on replacing that, but I want to convert most of the code that should modify such state to sans-io, so I don't break things accidentally.

[mzimbres]

Note for the future: Cancellation support will significantly simplify the health-check implementation and reduce the total number or parallel operations here.

[anarthal]

Yes, I saw that yesterday.