Re: [PATCH] Bug 3389: Auto-reconnect for tcp access_log

On 17/04/2013 3:41 a.m., Alex Rousskov wrote:
> On 04/16/2013 01:39 AM, Amos Jeffries wrote:
>> 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 code and detailed development history are also available at
>>>>> https://code.launchpad.net/~squid/squid/bug3389
>>>> access_log logger=<module>:<place> [option ...] [acl acl ...]
>>>> access_log <module>:<place> [<logformat name> [acl acl ...]]
>>>> access_log none [acl acl ...]
>
>>>> 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.
> Yes, but it is a weakness of the current syntax, not its strength.

I know. It is a weakness you are not solving in scope of the TCP module
upgrade, so needs to be taken advantage of rather than adding more
complexity to workaround it. Minimal impact changes remember.

If you have some way to alter the parser to remove that weakness without
breaking ACLs or adding a lot of complexity toa simple parser, please
propose it as a separate patch.

>
>> 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.
> I think our primary motivation should be "good configuration
> design"(succinct, easy to understand, easy to extend, etc.). Temporary
> coding difficulties ought to be secondary factors in deliberations about
> that design. Yesterday, we did not have the ability to undo strtok().
> Today we have that ability. Tomorrow, we may have a grammar-based parser
> that does not use strtok() and does not need [external/explicit] undo.

Yet we have to work with the existing code. Next years code is not going
to help this feature change go in today.

If we have a system to undo strtok() today then please do use that and
avoid the format nastiness entirely. Otherwise the format field or at
least some form of delimiter is still required.

PS. I think all of this discussion is a good sign that changing the
format fields and its behaviour from the slightly current inconvenient
but well-known style is a goodly sized project in its own right not
something to tack into scope of a log module upgrade.

Amos
Received on Thu Apr 18 2013 - 13:32:25 MDT