Re: "[PATCH] TCP logging" - Dhaval Varia

On Tue, Apr 13, 2010 at 12:05 PM, Dhaval Varia <dhavalkvaria_at_gmail.com> wrote:
> Dear Sir,
>
> This is dhaval varia. working on TCP Logging based on Udp logging facility.
>
> 1. I have completed the task client side (ModTcp.*) .daemon process left.
>
>     Please find the attached patch to view the changes.
>
> 2. Found bug in Udp logging facility in file ModUdp.cc
>     Solved and mentioned in attached patch.

Hi Dhaval,
  thanks for your contribution.

There are a few formal changes to be made before it can be considered,
unfortunately:

- the Udp fix should be kept separate from the ModTcp
- Makefile.in files should not be included in the patch, they are
generated form Makefile.am
- editor backup ( files ending in ~) files should not be included in the patch

Regarding the patch itself:
- why two debugs() statements in logfile_mod_tcp_open? They seem to
output the same text.
- Marking debugs() messages with a special string ("Dhaval from .." is
perfectly fine during early development, it's much less useful once
the code has matured.
- the error messages at line 935 of the patch seem odd.. do they apply
at all to TCP logging? They seem more like relics from a file logging
facility
- how does this logging facility handle the case of overload? Will it
block? Will it discard messages? There doesn't seem to be any facility
to have it handle backpressure.

Thanks again!

-- 
    /kinkie