okumin commented on code in PR #5639:
URL: https://github.com/apache/hive/pull/5639#discussion_r1950209422
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/HikariCPDataSourceProvider.java:
##########
@@ -55,6 +56,7 @@ public DataSource create(Configuration hdpConfig, int
maxPoolSize) throws SQLExc
Properties properties = replacePrefix(
DataSourceProvider.getPrefixedProperties(hdpConfig, HIKARI));
long connectionTimeout = hdpConfig.getLong(CONNECTION_TIMEOUT_PROPERTY,
30000L);
+ long maxLifetime = hdpConfig.getLong(MAX_LIFETIME, 3600000L);
Review Comment:
Thanks for reviewing. I answer the questions of myself and yours first.
My question 1: What's the max lifetime?
Let's see [Configuration (knobs, baby!) in the official
document](https://github.com/brettwooldridge/HikariCP?tab=readme-ov-file#gear-configuration-knobs-baby).
> This property controls the maximum lifetime of a connection in the pool.
An in-use connection will never be retired, only when it is closed will it then
be removed. On a connection-by-connection basis, minor negative attenuation is
applied to avoid mass-extinction in the pool. We strongly recommend setting
this value, and it should be several seconds shorter than any database or
infrastructure imposed connection time limit. A value of 0 indicates no maximum
lifetime (infinite lifetime), subject of course to the idleTimeout setting. The
minimum allowed value is 30000ms (30 seconds). Default: 1800000 (30 minutes)
My question 2: What's the leak detection threshold?
> This property controls the amount of time that a connection can be out of
the pool before a message is logged indicating a possible connection leak. A
value of 0 means leak detection is disabled. Lowest acceptable value for
enabling leak detection is 2000 (2 seconds). Default: 0
From the same document, [Configuration (knobs, baby!) in the official
document](https://github.com/brettwooldridge/HikariCP?tab=readme-ov-file#gear-configuration-knobs-baby).
[HikariCP makes a warning log when the leak threshold is
violated](https://github.com/brettwooldridge/HikariCP/blob/dev/src/main/java/com/zaxxer/hikari/pool/ProxyLeakTask.java).
My question 3: Why did we change our default value?
HIVE-25482 says the original author faced a corner case and suggests making
it configurable and configuring the default value.
My question 4: When did 1 hour become invalid?
[This is the merge commit of
HIVE-25482](https://github.com/apache/hive/commit/c066d5d2b5098bb7cd4b7a5500cc076c32dccf1e).
[We were using HikariCP 2.6.1 at that
time](https://github.com/apache/hive/blob/c066d5d2b5098bb7cd4b7a5500cc076c32dccf1e/pom.xml#L150).
The [MAX_LIFETIME of 2.6.1 is 30
minutes](https://github.com/brettwooldridge/HikariCP/blob/HikariCP-2.6.1/src/main/java/com/zaxxer/hikari/HikariConfig.java#L56)
and [it already had the same
validation](https://github.com/brettwooldridge/HikariCP/blob/HikariCP-2.6.1/src/main/java/com/zaxxer/hikari/HikariConfig.java#L890-L895).
So, our 1 hour has been invalid since we merged HIVE-25482.
> what if the max lifetime is equal to the leak detection, will the rule for
leak detection not take effect as the max lifetime takes first?
I guess it is possible, though I've not tested it.
> After checking a bit further the code, I get the impression that
getPrefixedProperties block is sufficient to read any user defined Hikari
values from the conf. I don't understand what's the purpose of using/adding
explicit getters/setters per property.
I guess we wanted to overwrite the default value, but I'm not sure. Let me
test some cases
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
