Update HikariCP and other dependencies #1821
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marcospereira
changed the title
marcospereira
commented
Dec 5, 2017
| c.getBooleanOpt("autoCommit").foreach(hconf.setAutoCommit) | ||
|
|
||
| val numThreads = c.getIntOr("numThreads", 20) | ||
|
|
||
| hconf.setConnectionTimeout(c.getMillisecondsOr("connectionTimeout", 1000)) |
There was a problem hiding this comment.
This is one of the places where Slick is not using HikariCP defaults. While Slick uses 1 second, Hikari defaults to 30 seconds.
Should Slick respect HikariCP default?
marcospereira
commented
Dec 5, 2017
| .map("TRANSACTION_" + _) | ||
| .foreach(hconf.setTransactionIsolation) | ||
|
|
||
| hconf.setValidationTimeout(c.getMillisecondsOr("validationTimeout", 1000)) |
There was a problem hiding this comment.
This is another place where Slick is not using HikariCP defaults (5 seconds).
marcospereira
commented
Dec 5, 2017
| // `initializationFailTimeout`. See HikariCP docs for more information: | ||
| // https://github.com/brettwooldridge/HikariCP#infrequently-used | ||
| // But we still respect the value if it configured. | ||
| c.getBooleanOpt("initializationFailFast").foreach(hconf.setInitializationFailFast) |
There was a problem hiding this comment.
Effectively avoids the log warning. Also, initializationFailTimeout overrides it if both are present.
marcospereira
commented
Dec 5, 2017
| * database is still alive. It is database dependent and should be a query that takes very | ||
| * little processing by the database (e.g. "VALUES 1"). When not set, the JDBC4 | ||
| * `Connection.isValid()` method is used instead (which is usually preferable).</li> | ||
| * <li>Essentials: |
There was a problem hiding this comment.
Changes the docs organization and add docs to the HikariCP configurations added. But most of the content is untouched.
|
@marcospereira unfortunately, this now conflicts with |
Most relevant changes: 1. initializationFailFast is now handled as an optional configuration. If it is not present, then no warning is printed. 2. Some keys such as `url`, `user` and `driver` now supports the HikariCP names like `jdbcUrl`, `username` and `driverClassName`. 3. Change documentation to separate required, frenquently used and infrenquetly used configurations.
It is the current default behavior with `initializationFailFast` so `initializationFailTimeout` needs to be configured to -1 by default to keep the existing behavior.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes updates for the following dependencies: