[PATCH] HTTP: implement 'connect' and 'close' phases

[Maxim Dounin]

Hello!

On Mon, Nov 23, 2015 at 03:05:07PM +0000, Alessandro Ghedini wrote:

> On Fri, Nov 20, 2015 at 05:46:12pm +0300, Maxim Dounin wrote:
> > Hello!
> 
> Hi again,
> 
> > On Fri, Nov 20, 2015 at 09:45:53AM +0000, Alessandro Ghedini wrote:
> > 
> > > # HG changeset patch
> > > # User Alessandro Ghedini <alessandro at cloudflare.com>
> > > # Date 1447956026 0
> > > #      Thu Nov 19 18:00:26 2015 +0000
> > > # Node ID 9d265c320050a00ff24fa8d84371701e46147e8a
> > > # Parent  bec5b3093337708cbdb59f9fc253f8e1cd6d7848
> > > HTTP: implement 'connect' and 'close' phases
> > > 
> > > This patch adds the NGX_HTTP_CONNECT_PHASE and NGX_HTTP_CLOSE_PHASE
> > > phases.
> > > 
> > > Handlers for these phases are called when a connection is estabilished
> > > and when it is closed, and they take a ngx_connection_t as argument
> > > instead of ngx_http_request_t like the other phase handlers.
> > > 
> > > These can be useful for keeping track of TCP connections for debugging,
> > > monitoring and logging purposes, and can also be used to apply custom
> > > configurations (e.g. socket options).
> > > 
> > > This patch also adds a "ctx" field to ngx_connection_t, to be used by
> > > modules to store their own context structures (just like the ctx field
> > > in ngx_http_request_t).
> > 
> > Phases are request processing phases, and what you are trying to 
> > do doesn't looks like request processing.  Additionally, ctx in 
> > ngx_connection_t implies noticeable memory overhead for keepalive 
> > connections.
> 
> I think I can do without the ctx field. Would removing that help in getting
> this functionality (or something similar) merged?

Unlikely.  Though it's certainly a prerequisite.

> > Instead, consider:
> > 
> > - starting your processing at any request processing stage as 
> >   needed;
> > 
> > - using a connection pool cleanup handler if you want to track 
> >   connection termination;
> 
> The problem with the above is that the pool cleanup callbacks seem to be
> called too late, after the connection has already been closed, so I get an
> invalid socket (basically I need the socket to retrieve some information
> using getsockopt()).

If you want to collect something using getsockopt(), consider 
doing this in the log phase.

Instead of trying to introduce a generic mechanism you may want to 
focus on what you want to do and if it's possible to do this with 
already existing mechanisms.

-- 
Maxim Dounin
http://nginx.org/