search

vert-x3 / vertx-mysql-postgresql-client

This client is deprecated - use instead
https://github.com/eclipse-vertx/vertx-sql-client
Apache License 2.0
117 stars 59 forks source link

Release idle connections fixes #33 #118

Closed adamdickmeiss closed 5 years ago

adamdickmeiss commented 6 years ago

New configuration connectionReleaseDelay which has a default of 1 minute.

adamdickmeiss commented 6 years ago

On Tue, Jun 19, 2018 at 10:54 AM Joern Bernhardt notifications@github.com wrote:

@Narigo requested changes on this pull request.

Thanks for helping out @adamdickmeiss https://github.com/adamdickmeiss !

  1. Can you please add tests to see if the connections really get closed after the set amount of time?
  2. Can you please document the new option and especially the special "0 or less" meaning "no idle timeout"

OK.. Tried to add a test and some documentation.

https://github.com/folio-org/vertx-mysql-postgresql-client/commits/release-connection-pool-3-6-0

I also changed default behavior to 0 = no releasing.

But I want to stress that applications are probably faulty if they rely on "released" connections being kept alive.

The pool (reuse) is pure optimization , unless I'm seriously mistaking things :-)

  1. Did you sign the Eclipse contribution agreement? See the Vert.x wiki about contributions https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md#contributions

Signed.

Let me know if I should squash the changes into a new commit or something..

/ Adam

In src/main/java/io/vertx/ext/asyncsql/impl/pool/AsyncConnectionPool.java https://github.com/vert-x3/vertx-mysql-postgresql-client/pull/118#discussion_r196337988 :

public AsyncConnectionPool(Vertx vertx, JsonObject globalConfig, Configuration connectionConfig) { this.vertx = vertx; this.maxPoolSize = globalConfig.getInteger("maxPoolSize", DEFAULT_MAX_POOL_SIZE); this.maxConnectionRetries = globalConfig.getInteger("maxConnectionRetries", DEFAULT_MAX_CONNECTION_RETRIES); this.connectionRetryDelay = globalConfig.getInteger("connectionRetryDelay", DEFAULT_CONNECTION_RETRY_DELAY);

  • this.connectionReleaseDelay = globalConfig.getInteger("connectionReleaseDelay", DEFAULT_CONNECTION_RELEASE_DELAY);

As you have 0 as another special value - why not use that in order to not break code relying on over 1 minute?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-mysql-postgresql-client/pull/118#pullrequestreview-129868026, or mute the thread https://github.com/notifications/unsubscribe-auth/AIJB36gCTpNHCLUB96ws0qVhxUVHDHVZks5t-LxigaJpZM4Urj7a .

adamdickmeiss commented 6 years ago

Any news on this? Should I make a new pull request with one big commit?

julianladisch commented 6 years ago

@Narigo Please review adamdickmeiss' latest changed. What else should we improve?