Re: [PATCH] Shutdown runners

On 17/02/2014 2:56 p.m., Alex Rousskov wrote:
> On 02/16/2014 06:02 AM, Amos Jeffries wrote:
>> This begins the shutdown sequence cleanup by creating a Runners Registry
>> hook at the begining of the shutdown process.
>
> Before the [adjusted version of] this patch goes in, we need to finish
> the discussion started on the "[PREVIEW] cache_peer standby=N" thread.
> Specifically, we need to decide whether Runners Registry is kept as a
> general two-action API (activation + destruction) or adjusted to become
> the main.cc multi-action API (too many to list all actions here, but
> they include configuration, reconfiguration, shutdown, and exiting).
>
> As discussed on the standby thread, each model has its own advantages
> and disadvantages. The primary advantages for each model are:
>
> + Two-action: Can be used anywhere; not specific to main.cc.
> + Multi-action: Covers main.cc needs very well.
> The whole action sequence become easy to
> document and understand. Adding various main.cc events
> and actions for them is easy.
>
> And primary disadvantages are:
>
> - Two-action: Requires several classes to implement a group of
> related actions working on the same state. The "whole
> picture" view is difficult to see and understand.
> - Multi-action: Specific to main.cc needs.
>
>
> I suggest that we make the API specific to main.cc needs and add methods
> to cover all stages of main.cc lifetime, including configuration,
> reconfiguration, and shutdown. This is what the existing code
> essentially tries to do, with the increasing awkwardness.
>
> If future callers outside main.cc need to use a similar registration
> concept, we will add whatever registries are appropriate for the caller
> needs, including the old two-action API if that is what the caller
> really needs. At that time, we may actually decide to add a
> single-action API instead. Note how the current two-action API is
> already a poor match for some of the code, including the proposed
> AuthShutdownRr class that does not have an explicit destructor -- not
> every caller needs/has paired events that the current two-action API was
> designed for.
>
>
> If my suggestion to add shutdown() and other methods to runners is
> accepted, I can help with the corresponding adjustments (both trunk and
> the proposed patch).
>
> If my suggestion is rejected, I will post my thoughts on how to adjust
> the proposed patch for the current two-action API.

It seems we have discussed this same topic at least three times now in
relation to as many components of Squid.

I am in agreement with the multi-action design.

Amos
Received on Mon Feb 17 2014 - 03:42:25 MST