> On Mar 23, 2021, at 3:00 AM, Emmanuel Lécharny <elecha...@gmail.com> wrote:
>
>
> On 22/03/2021 19:41, Shawn McKinney wrote:
>>> On Mar 22, 2021, at 11:05 AM, Emmanuel Lécharny <elecha...@gmail.com> wrote:
>>>
>>> LDAP connections are quite stable. Again, this check is there to respect
>>> the commons-pool API contract. If the connection is dead, then doing this
>>> check will let the pool fetching another connection, which is good. OTOH,
>>> if you don't want to check connections while fetching one, then you are on
>>> your own (ie, deal with the consequences of a bad connection).
>> Again, I must disagree. Connections aren’t stable, subjected to env
>> conditions and we can never assume a connection is good.
>
> You *can* assume it's good, and react exceptionally if it's not. That the
> fastest way to deal with potential bad connections, if you want to avoid a
> check every time you pull a connection from the pool. (but see later for
> another option)
There are 150 locations in fortress core where an ldap connection is being
pulled from the pool. No way I want to revisit all of that code.
>
> Something in the API chain must do the job of testing and reconnect, every
> time it’s pulled from the pool.
>
> This is exactly what the testOnBorrow does ;-) But it's costly... (see later
> for another option)
>
>> Now, having said that, that’s exactly what I’m observing currently, with the
>> test on borrow flag turned off.
>> Let me explain the scenario:
>> 1. Start server
>> 2. Start client
>> This initializes a connection pool which creates and stores exactly 1
>> connection (min 1, max1 )
>> 3. Set breakpoint in client on pool.getConnection method
>> 4. Restart the server.
>> 5. Client performs ldap op which triggers the breakpoint on getConnection
>> 6. Server at this point still hasn’t any connections with the client. The
>> client ‘thinks’ it has connections in the pool, but these were broken on
>> step 4.
>> 7. Step over the getConnection which then triggers the commons pool to
>> execute:
>> ```
>> GenericObjectPool._factory.activateObject(latch.getPair().value)
>> ```
>> 8. A connection is made with the server, along with bind
>> ```
>> 6058e163 conn=1000 fd=14 ACCEPT from IP=127.0.0.1:35246 (IP=127.0.0.1:389)
>> [exec] 6058e163 conn=1000 op=0 BIND dn="cn=manager,dc=example,dc=com"
>> method=128
>> [exec] 6058e163 conn=1000 op=0 BIND dn="cn=manager,dc=example,dc=com"
>> mech=SIMPLE ssf=0
>> [exec] 6058e163 conn=1000 op=0 RESULT tag=97 err=0 duration=1.667ms
>> ```
>> 9. Client continues with its ldap op successfully and is never the wiser
>> that the connections were all forcibly closed on server restart.
>> This is EXACTLY what I want to see all of which is done without the test on
>> borrow eliminating the extraneous search on every connection retrieval.
>
> So that would mean we have some kind of 'retry' on connection operation: if
> it fails, then let's assume the connection is broken, and redo it with a
> fresh connection.
>
Yes, that’s what’s happening.
In the Commons pool lib, this is the block that gets executed:
``` this._factory.activateObject(latch.getPair().value);
if (this._testOnBorrow &&
!this._factory.validateObject(latch.getPair().value)) {
throw new Exception("ValidateObject failed");
}
```
In the first line above, activateObject, this code gets called, from our
AbstractPoolableLdapConnectionFactory class:
```
public void activateObject(LdapConnection connection) throws LdapException {
LOG.debug("Activating {}", connection);
if (!connection.isConnected() ||
!connection.isAuthenticated()) {
LOG.debug("rebind due to connection dropped on {}", connection);
this.connectionFactory.bindConnection(connection);
}
```
The code performs a rebind which renews the connection.
All of this with testOnBorrow being false!
So, I’m still scratching my head figuring why we need this secondary level that
is wasting a round trip to the server.
> The problem is that the pool is passive: it does not react to any connection
> event, like a connection closure. OTOH, when a connection is properly closed
> by the server (ie an NoticeOfDisconnect - aka NoD - is generated by the
> server), the connection should process it and die. Now we have an issue: the
> connection is just lying in the pool, not being used by any client, so there
> is a missing step: removing the connection from the pool in this very case.
> That can be something we can add to the current LDAP pool.
>
> Note that if the server does not send a NoD, you are screwed. There is no way
> to be sure that the connection is ok until you use it. OTOH, leveraging the
> setTestWhileIdle() could be a solution to partially workaround the issue.
Here you’ve lost me. I’m running the server in debug mode, inside a bash
shell, running in the foreground.
In my test I stop the server by ‘ctrl-c’ killing the shell.
The server is not reacting to this and sending a NoD.
—
Shawn
---------------------------------------------------------------------
To unsubscribe, e-mail: api-unsubscr...@directory.apache.org
For additional commands, e-mail: api-h...@directory.apache.org
