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

postgreSQLClient.rxGetConnection() has no timeout or maxWaitQueueSize #142

Closed iranna90 closed 5 years ago

iranna90 commented 5 years ago

If there are no connections available in the pool then postgreSQLClient.rxGetConnection() waits until a connection is available, If we have some slow query then the wait queue is growing indefinitely as waiters queue does not have any maxLimit and because of this we have response times going high/spiking as waiters queue grows, Is it a good idea to add 'maxWaitersQueueSize' parameter and any new connection request will be failed immediately after this size reached and caller can return a default response(operating in degraded mode), Or was there a specific reason we did not add this sizeLimit on waiters queue ?

iranna90 commented 5 years ago

Will it be fine If we can provide a fix for this resource leak we are having now in the ConnectionPool using "maxWaitQueueSize" configuration parameter to AsyncConnectionPool.

Solution: We are thinking of adding another configuration parameter "maxWaitQueueSize" same as maxPoolSize, and before adding the handler to waiter queue we can check whether the "queueSize < maxWaitQueueSize" then it will add the handler to the waiters queue.

By default it will have value as -1(unlimited queue size if not configured)

example code : public static final int DEFAULT_MAX_WAIT_QUEUE_SIZE = -1; // unlimited queue size private int queueSize = 0; private final int maxQueueSize = globalConfig.getInteger("maxWaitQueueSize", DEFAULT_MAX_WAIT_QUEUE_SIZE);

private synchronized void waitForAvailableConnection(Handler<AsyncResult> handler) { if (canAddWaiter()) { queueSize += 1; waiters.add(handler); return; } handler.handle( Future.failedFuture( new ConnectionPoolTooBusyException("Connection pool reached max wait queue size of " + maxQueueSize)) ); }

private synchronized boolean canAddWaiter() { return maxQueueSize < 0 || queueSize < maxQueueSize; }

And whenever a waiter gets connection and executes then we decrease the queueSize.

private synchronized void notifyWaitersAboutAvailableConnection() { Handler<AsyncResult> handler = waiters.poll(); if (handler != null) { queueSize -= 1; take(handler); } }

Let us know how can we proceed further if it is fine to push the branch and create a new pull request.

oshai commented 5 years ago

In my opinion, the best option going forward is to use the connection pool that is part of jasync. The pool has wait queue size configuration as well as other options.

iranna90 commented 5 years ago

Thanks for the information, And in which release are we planning of switching to connection pool from jasync-sql ?

oshai commented 5 years ago

I maintain jasync hence the suggestion. Since I'm not maintainig this lib I can only suggest it as a pull request. @codepitbull would you like to go that path?

iranna90 commented 5 years ago

@codepitbull are we planning to switch to connection-pool from jasync-sql? if not probably we can create a pull request to add maxWaitQueueSize configuration limit to existing AsyncConnectionPool

iseki0 commented 5 years ago

Before that, how to limit the request timeout or maxWaitQueueSize? So, if the pool is full, this method will not return until it gets a connection? Thanks, this is my first time use vert.x .

iranna90 commented 5 years ago

Even though connection pool is fullpostgreSQLClient.getConnection will return(It won't block). As we are providing callback handler while asking for a connection postgreSQLClient.getConnection(handler -> { **CODE** });, if there is a connection available in the pool then it will execute the handler code, but if there no connection in the pool(pool fully utilized) then we put these handlers in the waiters queue and return, once the connection becomes available in the pool then we start retrieving the handlers from the waiters queue and execute them. and right now this waiter queue is unbounded which causing some issues.

iranna90 commented 5 years ago

Closing the issue as the fix is merged to 3.6 and pull request was: https://github.com/vert-x3/vertx-mysql-postgresql-client/pull/143