Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement reconnections #108

Open
ernestojpg opened this issue Feb 16, 2018 · 5 comments
Open

Implement reconnections #108

ernestojpg opened this issue Feb 16, 2018 · 5 comments

Comments

@ernestojpg
Copy link
Contributor

Hi all!

It would be nice if the library could recover from temporary database outages, such as those which occur during a database restart or brief loss of network connectivity.

Because the underlying library from mauricio does not support this functionality yet, we could implement it easily in our AsyncConnectionPool.createConnection() method, just using a counter and Vert.x timers.

I would create a couple of new configuration properties. For example:

  • maxConnectionRetries
  • connectionRetryDelay

When our AsyncConnectionPool attempts to acquire an open Connection and fails, it will retry up to maxConnectionRetries times, with a delay of connectionRetryDelay between each attempt. If all attempts fail, any clients waiting for Connections from our AsyncConnectionPool will see an Exception, indicating that a Connection could not be acquired.

For keeping full compatibility with the current code, I would set maxConnectionRetries to 0 by default, that means that no reconnections will be done. If we set maxConnectionRetries to -1 our AsyncConnectionPool will attempt to acquire new Connections indefinitely.

What do you think? If you are happy with that I can create a pull request with the changes.

Regards.

@vietj
Copy link
Contributor

vietj commented Feb 16, 2018

it looks like a good feature to provide

@ernestojpg
Copy link
Contributor Author

Cool, I will try to implement it soon.

@vietj
Copy link
Contributor

vietj commented Feb 16, 2018

@ernestojpg perhaps you can look at Vert.x HttpClient pool that is now reusable. If we could reuse it, it would be great and keep only a single pool to maintain

ernestojpg added a commit to ernestojpg/vertx-mysql-postgresql-client that referenced this issue Feb 21, 2018
ernestojpg added a commit to ernestojpg/vertx-mysql-postgresql-client that referenced this issue Feb 21, 2018
ernestojpg added a commit to ernestojpg/vertx-mysql-postgresql-client that referenced this issue Feb 21, 2018
@ernestojpg
Copy link
Contributor Author

Hi @vietj , sorry for the delay in answering.

I'm not 100% sure, but yes, we could probably use the new http client pool for this service as well, but it would require a big refactoring of the "vertx-mysql-postgresql-client" service first (for using the new pool), and then implementing the reconnections logic in the new pool without breaking anything else. I don't know if I will have so much time :)

For now, I have implemented the change easily in the current pool, and I will try to implement the new pool in the future. Let me know what you think.

Regards.

@vietj
Copy link
Contributor

vietj commented Feb 21, 2018

@ernestojpg ok

ernestojpg added a commit to ernestojpg/vertx-mysql-postgresql-client that referenced this issue Feb 28, 2018
ernestojpg added a commit to ernestojpg/vertx-mysql-postgresql-client that referenced this issue Feb 28, 2018
Signed-off-by: Ernesto J. Perez <[email protected]>
Narigo added a commit that referenced this issue Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants