[squid-dev] [PATCH] helper queue polishing

[Amos Jeffries]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> 

> On 11/04/2014 03:52 AM, Amos Jeffries wrote:
>> On 4/11/2014 8:05 a.m., Tsantilas Christos wrote:
>>> This patch try to polish helpers queue to: 1. Make the queue
>>> limit configurable, with the default set to 2*n_max. 2. Move
>>> common queue limit checks inside helper.cc. 3. Make all code to
>>> use those new helper features.
>>> 
>>> A first discussion for such changes done on squid-dev on a 
>>> mail-thread under the subject "[PATCH] Prevent external_acl.cc 
>>> "inBackground" assertion on queue overloads" : 
>>> http://www.squid-cache.org/mail-archive/squid-dev/201301/0083.html
>>>
>>>
>>> 
For more informations please read documentation in patch preamble.
>>> 
>>> This is a Measurement Factory project
>>> 
>> 

>> 
>> * need to document how the redirector/storeid helper bypass has
>> changed. - when bypass AND queue-size>0 are configured a queue
>> exists, older config the queue was always bypassed.
> 
> It should be ok now... I document these changes in patch preameble,
> and I will add them when I commit the patch to trunk. Do we need
> such documentation on release-3.6.sgml file? I put here only basic
> documentation.
> 

Since you added the backward compat the behaviour should not have
changed unless explicitly configured.

So I dont think we need to release notes this change. But updating
their cf.data.pre text to make it clear how the queue parameter being
used affects bypass would be good.

>> 
>> 
>> in src/helper.cc: (optional but desirable to reduce code) * why
>> are you retaining the legacy helperSubmit() now that helpers all 
>> have a members? - the helpers still using old queue limits should
>> just have queue_size hard-coded to those limits and use the new
>> API methods.
>> 
>> * helperStatefulSubmit() is/was only used by Negotiate and NTLM
>> auth helpers which you have documented as supporting the new
>> queue-size. So there should be no need for helperStatefulSubmit()
>> to exist now. - you can probably remove both these wrappers by
>> making submit() virtual, and both submit() and trySubmit()
>> receive a 'lastserver' parameter (default of NULL for it).
> 
> I prefer to not make these changes now... The helper and
> statefullhelper classes need to be "more" c++ classes and their
> code merged if possible. But this is an other project....
> 

Okay.

>> 
>> 
>> Also: * missing doc/release-notesic /release-3.6.sgml entries for
>> the updated config directives.
> 
> I put only basic documentation. I did not put the helper behaviour
> changes. Someone can find them when reads the squid.conf. Is it
> OK?
> 

Looks fine. Thank you.


+1 for commit.

Amos
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)

iQEcBAEBAgAGBQJUXH+1AAoJELJo5wb/XPRjpjsH/i5EFdNFRxTW4FLWxhvg5LQX
wVx77YpPfc1hHA7ltEcmX0k0E77kCUljzalF4ouzLKBMcD2tPqqrhwYQRKjJIUk3
ogfaetaz7jr9hD5xo8+JtdaejnhpJ6j/smVmQBw1Hqc1tzDqNYJn/aIktCiNlC56
SN+ljj368otNd/2mJ3QCURTVTIQV+pp1nFoJ1498tL4f7n/QxC8lB/1Y23rhCERl
0S1gM/sfvVNiJYx97lA1kSHB6EtKGupl49CE8B7YtKddxnjdc+eED8GuDJT4YVZo
T43x3Orua3fhYdKD2mZtV/Xl3aX1crjelAOy/oECwYM3t+xnmJi2hnO+PxvH7Wg=
=tdWB
-----END PGP SIGNATURE-----