vert.x icon indicating copy to clipboard operation
vert.x copied to clipboard

Add `remove` to connection pool `Lease`

Open kdubb opened this issue 2 years ago • 6 comments

Implements #4680

kdubb avatar Apr 24 '23 02:04 kdubb

it seems that this operation can break pool invariants, e.g if the connection has several leases, when another lease calls recycle() then no operation should be done since the slot might hold a different connection, e.g it could call slot.usage-- and we assume this is possible because it has increased the slot usage before with the same connection

vietj avatar Apr 24 '23 07:04 vietj

as I understand you would like to mark a connection as stale so it should never be borrowed again from the pool and eventually becomes removed from the pool to let another connection to be created ?

vietj avatar Apr 24 '23 07:04 vietj

This probably should have been a draft because I don't quite understand all the inner workings of the pool slots.

as I understand you would like to mark a connection as stale so it should never be borrowed again from the pool and eventually becomes removed from the pool to let another connection to be created ?

Essentially yes, we need a way to communicate to the pool that the connection should be removed.

kdubb avatar Apr 24 '23 07:04 kdubb

I think then we need a new shutdown operation on ConnectionPool that performs this that will set a shutdown flag on the slot and have the pool not check to lease this again when the flag is true, pretty much like:

void shutdown(Predicate<C> pred, Supplier<C> closer)

It would be called like pool.shutdown(conn -> conn.isValid(), conn -> conn.close())

when a connection is marked as shutdown, the slot.usage can only decrease and when it reaches 0 then it will be simply removed from the pool an then closed, another connection can be created to satisfy the waiters then.

vietj avatar Apr 24 '23 07:04 vietj

@vietj @tsegismont I tried to implement this idea as a pre-cursor to maintaining minIdle connections in the pool.

Basically,

  1. Add a maxTttlMs option to the pool
  2. Add a inactive flag to the slot (boolean), and createTimeMs field to the slot.
  3. createTimeMs is initialized when a connection is created in a slot
  4. In recycle, check if the maxTtlMs has expired for the slot. If yes, and if the slot usage is 0, remove the connection and set inactive = true. If not, only set inactive = true.
  5. In acquire, add a check to the Selector to not select any slot that has inactive = true
  6. When a new connection is added to the slot, change inactive back to false, reset createTimeMs

Then, during pool initialization and during remove, we need to call a method that refreshes the pool with minIdle connections (not part of the gist).

If this looks good to you, I can create a PR. Please let me know.

https://gist.github.com/chandramouli-r/f8adf3b765919ceb54dea9126a2d97f2

chandramouli-r avatar Jun 12 '23 02:06 chandramouli-r

@vietj @tsegismont create a draft PR for this: https://github.com/eclipse-vertx/vert.x/pull/4736 (the added API can be used in SqlConnectionPool for checking maxLifetime when a connection is returned back to the pool).

chandramouli-r avatar Jun 13 '23 03:06 chandramouli-r