(Mostly thinking out loud)
Is there any downside in us wrapping the NetworkLdapConnection returned by
the LdapConnectionPool to call releaseConnection instead of close?
Possibly a new `PooledLdapConnection` that _basically_ delegates everything
except the `close` method?
That would allow backward compatibility with the current
`releaseConnection()` usage. Possibly not with `close()` ( though I'm not
sure that's a valid code path?)
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
>
>
