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

HIVE-28749: The default hikaricp.leakDetectionThreshold is not valid #5639

Merged
merged 3 commits into from
Feb 14, 2025

Conversation

okumin
Copy link
Contributor

@okumin okumin commented Feb 9, 2025

What changes were proposed in this pull request?

HicariConfig's leak detection threshold has to be smaller than the max lifetime. Our default leak detection threshold is greater than the maximum lifetime defined on the HikariCP side and is disabled by default. Please check the JIRA for details.

https://issues.apache.org/jira/browse/HIVE-28749

Why are the changes needed?

Make our default value effective.

Does this PR introduce any user-facing change?

No.

Is the change a dependency upgrade?

No.

How was this patch tested?

  • I verified the warning disappears with default configurations
  • I tested hikaricp.maxLifetime was configurable in my local machine

Copy link
Member

@zabetak zabetak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a minor comment but otherwise LGTM! Feel free to submit once you get a green run.

Properties properties = replacePrefix(
DataSourceProvider.getPrefixedProperties(hdpConfig, HIKARI));
long connectionTimeout = hdpConfig.getLong(CONNECTION_TIMEOUT_PROPERTY, 30000L);
long leakDetectionThreshold = hdpConfig.getLong(LEAK_DETECTION_THRESHOLD, 3600000L);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a new unit test explains, the default connection timeout is originally 30 seconds.
We also decided to use the default leak detection threshold. It doesn't change the behavior because the current value is invalid, and the detection is disabled.

@okumin okumin merged commit b395fc7 into apache:master Feb 14, 2025
4 checks passed
@okumin okumin deleted the HIVE-28749-hikari branch February 14, 2025 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants