> On Feb 21, 2024, at 4:21 PM, Emmanuel Lécharny <elecha...@gmail.com> wrote:
>
> On 21/02/2024 22:09, Brian Demers wrote:
>> (Mostly thinking out loud)
>> Is there any downside in us wrapping the NetworkLdapConnection returned by
>> the LdapConnectionPool to call releaseConnection instead of close?
>
> We could do that, to some extent. LdapNetworkConnection can be extended, so
> we could add the Closeable interface to it, overriding the close() method and
> adding something like a shutdown() method that would call the parent's
> close() method:
>
> public class PooledLdapNetworkConnection extends LdapNetworkConnection
> implements AutoCloseable
> {
> LdapConnectionPool pool;
> LdapConnection kdapConnection;
>
> public PooledLdapNetworkConnection( LdapConnection ldapConnection,
> LdapConnectionPool pool )
> {
> this.ldapConnection = ldapConnection;
> this.pool = pool;
> }
>
> @Override
> public void close()
> {
> try
> {
> pool.releaseConnection( ldapConnection );
> }
> catch ( LdapException le )
> {
> // Nothing to do here
> }
> }
>
> public void shutdown()
> {
> ldapConnection.close();
> }
> }
>
> The problem here is that the connection is never associated to the pool
> unless you can have it pushed into the pool, which is normally done by the
> factory and specifically the makeObject() method;
>
> Here we have an issue:
> - we want to be able to get a connection from the pool, which extends
> Closeable
> - such an object is created by the factory which does create
> LdapNetworkConnection instances.
>
> So we should create a new LdapConnectionFactory that does instanciate
> PooledLdapNetworkConnection instances in its newLdapConnection() function,
> which get called by the factory.
>
> There is some plumbing to be done, and it will certainly not be
> straightforward, but still.
>
> Even simpler, we could decide that the factory only creates pooleable
> instances (because all in all, this is the only reason we have such a
> factory).
>
Chiming in here with another option:
Fortress uses Apache Commons pooling mechanism. It doesn’t allow
try-with-resources (actually maybe that can be done), instead we use
try-catch-finally.
Proven, stable, tested in all sorts of conditions.
Here’s our wrapper that manages three kinds of connections:
https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/ldap/LdapConnectionProvider.java
You certainly wouldn’t need all of the stuff we have here.
—
Shawn
>
>
>> On Wed, Feb 21, 2024 at 12:57 PM Emmanuel Lécharny <elecha...@gmail.com>
>> wrote:
>>> Hi Brian,
>>>
>>> long story short: the LdapConnection interface (and implementation)
>>> already has a close() method that shutdowns the IO connection, so it's
>>> not possible to use the try-with-resources feature, because that would
>>> have meant changing the LDAP API to make the close() method actually
>>> close the LDAP session, not the IO connection...
>>>
>>> So we added the releaseConnection() method which puts back the
>>> connection into the pool, but needs to be called explicitly.
>>>
>>> yes, it's a bit of a burden...
>>>
>>> Keep in mind that the LDAP API has been defined in 2006, 8 years before
>>> try-with-resources was included in Java 8 :/
>>>
>>>
>>> On 21/02/2024 17:59, Brian Demers wrote:
>>>> I'm hacking away on a SCIMple + ApacheDS + LDAP API example.
>>>> I haven't used the LDAP API project before, but the docs have been a big
>>>> help!
>>>>
>>>> One thing I'm struggling with is the way connection pooling _should_ be
>>>> used.
>>>>
>>>> For example, a non-pooled request could be created doing something like
>>>> this:
>>>>
>>>> ```
>>>> try (LdapConnection connection = new LdapNetworkConnection( "localhost",
>>>> 389 )) {
>>>> // do something with connection
>>>> }
>>>> ```
>>>>
>>>> The connection will close and everyone is fine.
>>>>
>>>> When using a connection pool the pattern is a little different
>>>>
>>>> I was doing something like this:
>>>>
>>>> ```
>>>> try (LdapConnection connection = connectionPool.getConnection()) {
>>>> // do something with connection
>>>> }
>>>> ```
>>>>
>>>> But after making that change, I started seeing tests hang, after a few of
>>>> them had run and used a connection (possibly connections were locked)
>>>>
>>>> I stumbled on one of the connection pooling tests that looked like I
>>> should
>>>> be calling `connectionPool.releaseConnection(connection)`
>>>> That seemed to fix my issue (this should be mentioned here,
>>>>
>>> https://directory.apache.org/api/user-guide/2.1-connection-disconnection.html
>>> ,
>>>> I submit a doc fix once this thread is resolved)
>>>>
>>>> So that ends up being something like this
>>>> ```
>>>> LdapConnection connection = connectionPool.getConnection()
>>>> try {
>>>> // do something with connection
>>>> } finally {
>>>> connectionPool.releaseConnection(connection)
>>>> }
>>>> ```
>>>>
>>>> This solution is fine, but... I was expecting the pooled connection to
>>>> release the connection instead of closing it.
>>>>
>>>> I hacked around this by extending the LdapConnectionPool and forcing a
>>>> release instead of a close.
>>>> Quick & dirty with a reflection Proxy:
>>>> https://gist.github.com/bdemers/ec2da73f8496fe1cc673619b84f3f450
>>>>
>>>> After this, my pooled connections could be used the same as non-pooled
>>>> connections, which was my goal, but...
>>>>
>>>> Now for the naive question.
>>>> Is this a good idea? Are there times when I would want to close the
>>>> connection instead of releasing it?
>>>> Should we think about adding something like this to LdapConnectionPool,
>>> or
>>>> is this a doc issue?
>>>>
>>>> Thanks
>>>> -Brian
>>>>
>>>
>>> --
>>> *Emmanuel Lécharny* P. +33 (0)6 08 33 32 61
>>> elecha...@apache.org
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: api-unsubscr...@directory.apache.org
>>> For additional commands, e-mail: api-h...@directory.apache.org
>>>
>>>
>
> --
> *Emmanuel Lécharny* P. +33 (0)6 08 33 32 61
> elecha...@apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: api-unsubscr...@directory.apache.org
> For additional commands, e-mail: api-h...@directory.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscr...@directory.apache.org
For additional commands, e-mail: api-h...@directory.apache.org
