On 9/08/2016 3:14 a.m., Eduard Bagdasaryan wrote:
> This patch allows to configure Squid so that it reject overloaded helper
> requests instead of [default] crashing.
>
> Added on-persistent-overload=action option to helpers. Helper overload
> is defined as running with an overflowing queue. Persistent helper
> overload is [still] defined as being overloaded for more than 3 minutes.
>
> The default behavior is unchanged(*) -- Squid worker dies with a fatal
> error at the attempt to submit a new request to a persistenly overloaded
> helper. This default behavior can also be configured explicitly using
> on-persistent-overload=die.
>
> With on-persistent-overload=err, when dealing with a persistently
> overloaded helper, Squid immediately drops the helper request and sends
> an empty response to the caller which should handle that empty response
> as an error. Squid informs the admin when it starts and when it stops
> dropping helper requests due to persistent overload.
>
> The code had conflicting notions of an "overloaded helper". The external
> ACL helper, the URL rewriter, and the store ID code used queueFull() to
> test whether the new request would overflow the queue (and, hence,
> overload the helper), but queueFull() itself did not check whether the
> queue was full! It checked whether the queue was already overflowing.
> This confusion resulted in that code scheduling one extra helper request
> before enabling bypass. The code and its documentation are now more
> consistent (and better match the "overload" terminology used by the new
> configuration option, which also feels better than calling the helper
> "full").
>
> (*) Resolving the above confusion resulted in minor (one request)
> differences in the number of helper requests queued by Squid for
> external ACL, URL rewriting, and store ID helpers, with the adjusted
> behavior [better] matching the documentation.
>
> Regards,
> Eduard.
>
Thank you
in src/cf.data.pre:
* s/temporary exceed/temporarily exceed/
* For things like:
"on-persistent-overload option, described below, applies"
- please remove the above/below wording. The online config
documentation uses separate pages for options, and the config generator
can re-order directives sometimes. There is no dependable above/below to
refer to.
- above/below occurs several times in the new texts.
* the lines documenting 'die' and 'err' action look a bit squashed up.
Please:
- move "Two actions are supported:" to the start of a new line
- empty lines for clarity on listed entries.
- use 2 space indent on the action.
- tab indent all the description lines to make sure that all lines
align on their left side.
- remove the colons, that avoids anyone being confused about whether
they are part of the options name.
It should look like this:
"
.....
Two on-persistent-overload actions are supported:
die ...
err ...
...
"
in src/helper.cc
* "whether at least one more request can be successfully submitted"
- looks wrong documentation for the overloaded() method.
- the true output happens when the queue *is* past full, not when one
more requests can be added.
* why is helper::SubmissionFailure() a member of helper?
- it does not use any of *this class members.
- you could make it a templated local static function:
template<class Type> static void
SubmissionFailure(const Type hlp, ...
* helper::SubmissionFailure is also changing what was previously
Helper::Unknown result codes to Helper::Error.
- Helper::Error is one of the helper output codes, it means success.
Obviousy when hlp == NULL, there is no success going on.
- please revert to sending helper::Unknown code.
Aso, can we please add a "wait" action as well. Which means just keep
queueing things and ignore the overload.
Cheers
Amos
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
