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.
