RE: Comment on conn.c/conn_pool_put ()

I think that the implementation is correct - notice that conn_pool_put() puts the conenctions in a list, and make sure that only one such list exist for a key by using dict_get first and doing put only for a new key.
Further more, it is important to be able to hold many connection to the same host and port, for example - to access several resources at once on the same HTTP server. the conn_pool_get() implementation is good, as conn_pool_put() appends new connections to the end of the list while conn_pool_get() fetches connections from the top of the list, checking each one to see if its still alive until a live one is found (the oldest live connection) which is returned. so connections cycle in a round robin fashion. 
Maybe what is neccessary is a thread to watch over the connection pool and recycle a connection once the server drops it. else its a problem with the client implementation, if it opens many connections but never use them - the underlying connection library should not get involved in such matters.

The only gripe I have with conn_pool_*() is that these are not in the conn.c/h files. I think it's a nice functionality which shouldn't be limited to HTTP. and also the fact that conn_pull_get() can open a new connection from a specified IP address, but if it gets a connection from the pool it doesn't care from which IP it originates as long as the target is valid. I think the keys need to include the source address too.

--
Oded Arbel
m-Wise Mobile Solutions

[email protected]
Mobile: +972-67-340014
Tel: +972-9-9581711 (ext: 116)

::..
Don't let people drive you crazy when you know it's in walking distance.


> -----Original Message-----
> From: Paul Keogh [mailto:[email protected]]
> Sent: Thursday, June 20, 2002 7:42 PM
> To: Kannel Development List (E-mail)
> Subject: Comment on conn.c/conn_pool_put ()
> 
> 
> 
> This function (see below) can add *multiple* Conns to the pool
> based on the same key. Why is this needed ? It may make more
> sense to use dict_put_once() rather than dict_put() to
> ensure that the Conns added to the pool are unique. 
> 
> For this function to make sense, the opposite function should
> randomly select a Conn from the returned list (conn_pool_get()).
> It does'nt do this, instead it takes the first one that is still
> alive.
> 
> The reason that this is an issue is that multiple Conns will hold
> open resources on a server (notably socket descriptors) but will
> never be used by the client. I have had a situation where the PPG
> ran out of descriptors under a stress test from test_ppg.
> 
> Thanx,
> 
> static void conn_pool_put(Connection *conn, Octstr *host, int port)
> {
>     Octstr *key;
>     List *list;
>     
>     mutex_lock(conn_pool_lock);
>     key = conn_pool_key(host, port);
>     list = dict_get(conn_pool, key);
>     if (list == NULL) {
>     	list = list_create();
> 	dict_put(conn_pool, key, list);
>     }
>     list_append(list, conn);
>     octstr_destroy(key);
>     mutex_unlock(conn_pool_lock);
> }
>  
> 
>