Re: [API] LDAP Connection Pool - releasing/closing request
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).
On Wed, Feb 21, 2024 at 12:57 PM Emmanuel Lécharny
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
Re: [API] LDAP Connection Pool - releasing/closing request
(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
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
>
>
Re: [API] LDAP Connection Pool - releasing/closing request
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
[API] LDAP Connection Pool - releasing/closing request
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
