On 16/04/2013 5:30 p.m., Alex Rousskov wrote:
On 04/15/2013 09:33 PM, Amos Jeffries wrote:
On 13/04/2013 4:43 a.m., Alex Rousskov wrote:
The attached trunk patch fixes several problems with TCP module for
access_log. The module is unusable in production without these changes
if there is a chance the TCP logger becomes unavailable or TCP bandwidth
becomes temporary restricted . Nathan Hoad is the primary author of
these improvements. Factory helped Nathan during last stages of the
project. The code passed initial lab tests.
The code and detailed development history are also available at
https://code.launchpad.net/~squid/squid/bug3389
1) The implementation of the access_log config parser does not match the
documentation or this description. It currently implements the syntax:
access_log logger=<module>:<place> [option ...]
access_log <module>:<place> [<logformat name> [acl acl ...]]
access_log none [acl acl ...]
... the new options code will halt with self_destruct() and "Unknown
access_log option" if anything like ACLs occur.
My bug. Documentation was correct, but I missed testing this common
case. Very embarrassing. Implementation fixed in the attached patch.
I propose keeping the access_log syntax largely unchanged by simply
inserting [options] before the format field.
Making the directive syntax:
access_log module:place [options] [ format [acls ...] ]
The parser can exit the options parse loop when either EOL or the log
format name field is identified. There is also no problems introduced
about multiple logger= or format= options being listed. This is fully
backward-compatible and does not loose any popular features.
We can do that, but it requires an explicit format name to use ACLs
The existing config does as well.
My first thought was to make format an option too and detect first
unknown option as an ACL. But the ACL parsing is a separate function
that does not support un-doing a strtok(). Leaving the format field
untouched for now makes it useful as a delimiter token which can be used
to switch processing paths without having to un-do anything.
(for
no good reason that an admin may see) and makes the whole syntax less
uniform because some options have names and some do not (again, for no
good reason that an admin may see). I believe our proposed format is
better in those aspects, and the implementation should be
backwards-compatible without loosing any popular features as well (with
the attached fix).
I'm not sure what type of argument that is. Sounds like such admin need
to give in to their instincts and add the format names.
Amos
