On 08/09/2016 05:38 AM, Eduard Bagdasaryan wrote: > On 08/08/2016 02:17 PM, Amos Jeffries wrote: >> * 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.
> Reverted. Note that for helper::Unknown some callers print a bit > misleading error message, when requests are dropped, e.g.: > "ERROR: storeID helper returned invalid result code. Wrong helper?" and the documentation now lies: > + Two [on-persistent-overload] actions are supported: > + > + die Squid worker quits. This is the default behavior. > + > + err Squid treats the helper request as if it was > + immediately submitted, and the helper > immediately > + responded with an error. This action has no > effect on > + the already queued and in-progress helper > requests. Perhaps SubmissionFailure() should use Helper::Unknown when its hlp parameter is nil and Helper::Error in other cases? FYI: The original intent and the documentation say that we should return Helper::Error. Yes, Helper::Error means a "successfully parsed" helper response [with helper-specific semantics], and IIRC, that is what we wanted to relay to the helper-using code because that code has a better chance of doing the right thing when receiving Helper::Error (compared to handling Helper::Unknown). We also considered adding a new result code for overload but rejected that idea for several reasons, including the expectation that the helper-using code already has logic to handle Helper::Error and that logic will work for on-persistent-overload=err cases well. Please note that I am not advocating a _change_ in the current Squid default behavior. I am only questioning whether it is a good idea to return Helper::Unknown instead of Helper::Error with on-persistent-overload=err. Amos' arguments do not apply to that specific case AFAICT because there is no "change" in that specific case (it is a new behavior). Thank you, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
