Re: [RFC] sketch of a 'unified cancellation' approach

On 01/31/2013 10:08 AM, Rainer Weikusat wrote:
> All of Alex' objections to the cancelling are - of course - completely
> well-founded and this should really work in the way he described
> it. The problem is just "that's not how the existing code works".

Yes, but the four steps I sketched out recently are based on the
existing code and do _not_ require significant modifications of that
code: We already have cancellable async calls and we already have an
event queue that creates and uses them internally. We just need to
adjust the queue so that it accepts an async call from outside instead
of secretly creating it.

What you describe below goes a step further (more modifications, farther
from the existing code). This is not a critique of what you have
described -- just a note that it would be necessary to do something like
those four simpler steps anyway, and it is best to do them first, before
adding more/higher layers.

> I
> spent some time thinking how a 'nicer' cancellation subsystem with all
> desirable properties (both 'simple to use' and 'efficient to
> implement' could look like). Below is a sketch of that, feedback very
> much appreciated (May overlap with existing facilities. ATM, this is
> supposed to be abstract).
>
> - create abstract base class 'call queue' with virtual methods
> 'schedule' and 'cancel'
>
> - create another abstract base class 'call' with virtual methods
> 'take' and 'cancel'
>
> Something desiring to schedule a call would then create a call object
> and pass that as argument to the schedule method of a 'call queue' object.
> This object would do whatever is necessary to put this call onto its
> queue and invoke the 'take' method in order to take ownership of this
> call, passing a pointer to itself and 'some data item' as
> arguments. Assuming the original creator of the call wants the cancel
> it, it would invoke the cancel method of the call which would - in
> turn - invoke the cancel method of its current owner, passing a
> pointer to itself and the data item handed over when the 'call queue'
> took ownership. The 'call queue object' cancel method would then use this
> information in order to 'get rid of the call', whatever may be
> necessary to do that.

This is the right design IMO. We can indeed add the notion of "being
queued" to the existing async calls so that when AsyncCall::cancel()
method is called, the call object knows which "queue" to notify about
the cancellation. Some queues may not do anything about that
notification (e.g., the async queue), some will remove the call (e.g.,
the event queue).

If implemented on top of the four steps discussed earlier, it will
simplify callers that use events: Those callers will be able to do
call->cancel() instead of remembering to call eventDelete().

If you decide to work on this, please use the following plan:

1. Implement the four steps sketched earlier. They (or something like
them) are required to implement your design above anyway. Guide that
code through squid-dev review to the official commit.

2. Implement CallQueue, the abstract queue interface you described
above. Add the corresponding AsyncCall::enqueued(CallQueue *) and
AsyncCall::dequeued(CallQueue *) methods to AsyncCall, to maintain
"current queue" information inside async calls. The dequeued() call does
not really need a parameter, but I think it would be nice to assert that
the queue has not changed. The AsyncCall::cancel() method should call
queue->dequeue(*this) if queue is set. Post as PREVIEW.

3. Make existing async queue (AsyncCallQueue) and the existing event
queue (EventScheduler) children of CallQueue. Implement pure virtual
methods (enqueue and dequeue) in each child. Post as PATCH.

The above plan will probably need some adjustments, but I hope it has
enough specifics to point you in the right direction as far as existing
code modifications are concerned.

I think this plan will allow us to improve event handling without
getting bogged down in discussions about the need and the means for a
more efficient internal event queue implementation. And without
precluding that more efficient implementation in the future. It also has
two intermediate steps to minimize losses if there is a disagreement on
the specifics and something needs to be redone.

Thank you,

Alex.
Received on Thu Jan 31 2013 - 18:19:52 MST