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

Enable Hikari connection timeout to be set via environment variable #17425

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arnejduranovic
Copy link
Collaborator

@arnejduranovic arnejduranovic commented Feb 24, 2025

This PR updates application code, gradle, and terraform to allow the hikari connection timeout to be set via environment variable. This also fixes a bug where the configured connection timeout was not being respected.

Test Steps:
0. @devopsmatt to verify and validate TF changes

  1. Verify connection timeout property was not set on main branch by reading through the incident report that prompted this change: https://cdc.sharepoint.com/:w:/r/teams/ReportStream/_layouts/15/Doc.aspx?sourcedoc=%7B2E1169CA-B921-4CAA-B0E6-2BEF7443A1D7%7D&file=2025_02_18%20-%20Database%20Connectivity%20Issues.docx&action=default&mobileredirect=true
  2. Verify connection timeout is now set on this branch:
    Screenshot 2025-02-24 at 10 19 45 AM

Changes

  • Update Hikari config to respect connection timeout param
  • Allow Hikari connection timeout to be set via env var
    • Update gradle
    • Update TF

Copy link
Contributor

github-actions bot commented Feb 24, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

@@ -1414,10 +1415,7 @@ class DatabaseAccess(val create: DSLContext) : Logging {
config.addDataSourceProperty("cachePrepStmts", "true")
config.addDataSourceProperty("prepStmtCacheSize", "250")
config.addDataSourceProperty("prepStmtCacheSqlLimit", "2048")
config.addDataSourceProperty(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code was setting connection timeout to 60sec but in staging and prod the effective timeout was 30 seconds. So, I deleted this chunk of code and set the connection timeout via the connectionTimeout property, hoping this will fix the issue.

See the incident report linked in the PR description for more details.

@@ -102,6 +103,11 @@ val dbUrl = (
?: System.getenv(KEY_DB_URL)
?: "jdbc:postgresql://localhost:5432/prime_data_hub"
) as String
val hikariConfigTimeout = (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

default to 60 seconds if not set in env var

Copy link
Contributor

github-actions bot commented Feb 24, 2025

Test Results

1 280 tests  ±0   1 276 ✅ ±0   7m 30s ⏱️ -1s
  168 suites ±0       4 💤 ±0 
  168 files   ±0       0 ❌ ±0 

Results for commit 7e04e57. ± Comparison against base commit a06f23e.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Feb 24, 2025

Integration Test Results

0 files   -  60  0 suites   - 60   0s ⏱️ - 42m 46s
0 tests  - 428  0 ✅  - 418  0 💤  - 10  0 ❌ ±0 
0 runs   - 431  0 ✅  - 421  0 💤  - 10  0 ❌ ±0 

Results for commit 7e04e57. ± Comparison against base commit a06f23e.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@devopsmatt devopsmatt left a comment

Choose a reason for hiding this comment

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

Approving, but please test thoroughly before prod deployment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants