feat(core): enhance db configuration options #2285

jakedoublev · 2025-05-21T20:03:17Z

Proposed Changes

Expose connection/pooling configuration settings to global config settings

Checklist

I have added or updated unit tests
I have added or updated integration tests (if appropriate)
I have added or updated documentation

Testing Instructions

gemini-code-assist

Hello @jakedoublev, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request. This PR introduces enhanced configuration options for the database connection and pooling. The primary goal is to expose various pgxpool configuration settings, such as connection limits, idle timeouts, and health check periods, through the application's global configuration. This allows users to fine-tune database connection behavior without modifying code, improving flexibility and control over resource usage.

Highlights

Database Configuration: Adds new configuration fields under the db section for connection timeout and connection pool settings.
Connection Pooling: Exposes pgxpool specific settings like max_connection_count, min_connection_count, max_connection_lifetime_seconds, min_idle_connections_count, max_connection_idle_seconds, and health_check_period_seconds.
Documentation and Examples: Updates the Configuring.md documentation to list the new database configuration options and adds commented-out examples to opentdf-dev.yaml and opentdf-example.yaml.
Code Implementation: Modifies the database package (service/pkg/db) to include the new configuration struct (PoolConfig), parse the settings, and apply them when building the pgxpool.Config.
Testing: Adds a test case to ensure that the new pool configuration settings are correctly parsed and applied to the pgxpool.Config and do not incorrectly end up in the database connection string URL.

Changelog

Click here to see the changelog

docs/Configuring.md
- Added documentation for new database configuration options: connect_timeout_seconds and the nested pool settings.
- Updated the example configuration block to include the new database pool settings.
opentdf-dev.yaml
- Added commented-out examples for the new database connection timeout and pool configuration options.
opentdf-example.yaml
- Added commented-out examples for the new database connection timeout and pool configuration options.
service/pkg/db/db.go
- Imported the time package.
- Defined a new struct PoolConfig to hold database connection pool settings.
- Added ConnectTimeout (int) and Pool (PoolConfig) fields to the main Config struct.
- Updated the LogValue method for Config to include the new connection timeout and pool settings in the log output.
- Modified the buildConfig method to read and apply the ConnectTimeout and PoolConfig settings to the pgxpool.Config object.
service/pkg/db/db_test.go
- Renamed the test function Test_BuildConfig to Test_BuildConfig_ConnString.
- Added a new test case to Test_BuildConfig_ConnString to verify that pool configuration settings are not included in the generated connection string URL.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Database pool, a waiting queue,
Connections ready, fresh and new.
Set the max, set the idle time,
Performance tuned, a coding rhyme.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

docs/Configuring.md

gemini-code-assist

Code Review

This pull request introduces valuable enhancements to the database configuration by exposing more detailed connection and pooling settings. The changes are generally well-structured, with updates to the Go code, documentation, and example configuration files.

I've identified one critical issue related to the MinIdleConns configuration that will prevent compilation and needs to be addressed. Other changes, such as documentation updates and test additions, look good and improve the configurability of the database connection pool.

No specific style guide was provided, so the review is based on common Go language best practices (like Effective Go and Go Code Review Comments), standard Markdown, and YAML conventions.

Summary of Findings

Unsupported MinIdleConns Configuration: The code attempts to set MinIdleConns on the pgxpool.Config object, but this field does not exist in the pgx library's configuration struct. This will lead to a compilation error. The MinIdleConns field should likely be removed from PoolConfig and related documentation if it's not supported by the underlying library.
Markdown Formatting: In docs/Configuring.md, there are a few extra blank lines (lines 165-168 on the right side of the diff). This is a minor formatting issue and was not commented on directly due to review settings.

Merge Readiness

This pull request introduces useful database configuration enhancements. However, there is a critical issue identified in service/pkg/db/db.go concerning the MinIdleConns configuration that will prevent the code from compiling. This issue must be addressed before the PR can be considered for merging. Once the critical issue is resolved, the changes should be in good shape. As a reviewer, I am not authorized to approve pull requests, so please ensure further review and approval from authorized maintainers after addressing the feedback.

service/pkg/db/db.go

github-actions · 2025-05-21T20:07:47Z

Benchmark results, click to expand

Benchmark Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	464.9575ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	372.350514ms
Throughput	268.56 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	1m17.984831025s
Average Latency	778.118386ms
Throughput	64.12 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	4939
Failed Requests	61
Concurrent Requests	50
Total Time	1m7.831824771s
Average Latency	674.395491ms
Throughput	72.81 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	61 occurrences

Standard Benchmark Metrics Skipped or Failed

Copilot

Pull Request Overview

This PR enhances the database configuration by exposing connection timeout and pooling options at the global config level.

Introduce a new PoolConfig struct and ConnectTimeout in Config
Apply pool and timeout settings in buildConfig and log them in LogValue
Update tests, example YAMLs, and documentation to cover the new settings

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
service/pkg/db/db_test.go	Renamed test and added a case ensuring pool settings don’t alter the connection string
service/pkg/db/db.go	Added `PoolConfig`, `ConnectTimeout`, applied them in `buildConfig`, and updated `LogValue`
opentdf-example.yaml	Included commented pool and timeout settings in example
opentdf-dev.yaml	Included commented pool and timeout settings in dev config
docs/Configuring.md	Documented new pool and timeout fields under `db`

Comments suppressed due to low confidence (2)

service/pkg/db/db_test.go:46

Add assertions that verify parsed.MaxConns, parsed.MinConns, parsed.ConnConfig.ConnectTimeout, etc., are correctly applied when building the pool configuration.

// Pool config should not pollute connection string

docs/Configuring.md:148

The table header separator row is malformed, which will break rendering. Restore a proper separator line matching the column count (e.g., | -------------- | --------------------------------------------- | ----------- | --------------------- |).

| -------------------------------------- | ------------------------------------------------ | ----------- | ----------------------------------              |

service/pkg/db/db.go

github-actions · 2025-05-21T20:12:34Z

Benchmark results, click to expand

Benchmark Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	457.811475ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	372.363578ms
Throughput	268.55 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	1m18.319416561s
Average Latency	780.935296ms
Throughput	63.84 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	4942
Failed Requests	58
Concurrent Requests	50
Total Time	1m8.016620706s
Average Latency	677.063749ms
Throughput	72.66 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	58 occurrences

Standard Benchmark Metrics Skipped or Failed

service/pkg/db/db.go

github-actions · 2025-05-21T20:22:49Z

Benchmark results, click to expand

Benchmark Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	451.780771ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	363.743208ms
Throughput	274.92 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	1m16.934296502s
Average Latency	767.283245ms
Throughput	64.99 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	4948
Failed Requests	52
Concurrent Requests	50
Total Time	1m7.121434904s
Average Latency	666.122508ms
Throughput	73.72 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	52 occurrences

Standard Benchmark Metrics Skipped or Failed

docs/Configuring.md

github-actions · 2025-05-21T20:38:54Z

Benchmark results, click to expand

Benchmark Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	464.351527ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	364.368673ms
Throughput	274.45 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	1m13.484635547s
Average Latency	732.825849ms
Throughput	68.04 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	4950
Failed Requests	50
Concurrent Requests	50
Total Time	1m4.50060419s
Average Latency	641.571276ms
Throughput	76.74 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	50 occurrences

Standard Benchmark Metrics Skipped or Failed

🤖 I have created a release *beep* *boop* --- ## [0.5.3](service/v0.5.2...service/v0.5.3) (2025-05-22) ### Features * **authz:** authz v2 versioning implementation ([#2173](#2173)) ([557fc21](557fc21)) * **authz:** authz v2, ers v2 protos and gencode for ABAC with actions & registered resource ([#2124](#2124)) ([ea7992a](ea7992a)) * **authz:** export entity id prefix constant from entity instead of authorization service v1 ([#2261](#2261)) ([94079a9](94079a9)) * **authz:** subject mapping plugin support for ABAC with actions ([#2223](#2223)) ([d08b939](d08b939)) * bulk keycloak provisioning ([#2205](#2205)) ([59e4485](59e4485)) * **core:** add otel to opentdf services ([#1858](#1858)) ([53a7aa0](53a7aa0)) * **core:** Adds EC withSalt options ([#2126](#2126)) ([67b6fb8](67b6fb8)) * **core:** enhance db configuration options ([#2285](#2285)) ([ed9ff59](ed9ff59)) * **core:** New Key Index and Manager Plugin SPI ([#2095](#2095)) ([eb446fc](eb446fc)) * **core:** support onConfigUpdate hook when registering services ([#1992](#1992)) ([366d4dc](366d4dc)) * **core:** v2 ERS with proto updates ([#2210](#2210)) ([a161ef8](a161ef8)) * **policy:** actions crud service endpoints and proto validation ([#2037](#2037)) ([e933fa9](e933fa9)) * **policy:** actions service RPCs should actually hit storage layer CRUD ([#2063](#2063)) ([da4faf5](da4faf5)) * **policy:** add enhanced standard/custom actions protos ([#2020](#2020)) ([bbac53f](bbac53f)) * **policy:** Add platform key indexer. ([#2189](#2189)) ([861ef8d](861ef8d)) * **policy:** consume lib/identifier parse function ([#2181](#2181)) ([1cef22b](1cef22b)) * **policy:** DSPX-1018 NDR retrieval by FQN support ([#2131](#2131)) ([0001041](0001041)) * **policy:** DSPX-1057 registered resource action attribute values (DB + Service implementation) ([#2191](#2191)) ([6bf1b2e](6bf1b2e)) * **policy:** DSPX-1057 registered resource action attribute values (protos only) ([#2217](#2217)) ([6375596](6375596)) * **policy:** DSPX-893 NDR define crud protos ([#2056](#2056)) ([55a5c27](55a5c27)) * **policy:** DSPX-898 NDR database schema ([#2055](#2055)) ([2a10a6a](2a10a6a)) * **policy:** DSPX-901 NDR database crud ([#2071](#2071)) ([20e0a5f](20e0a5f)) * **policy:** DSPX-902 NDR service crud implementation (2/2) ([#2066](#2066)) ([030ad33](030ad33)) * **policy:** DSPX-902 NDR service crud protos only (1/2) ([#2092](#2092)) ([24b6cb5](24b6cb5)) * **policy:** Finish resource mapping groups ([#2224](#2224)) ([5ff754e](5ff754e)) * **policy:** GetMatchedSubjectMappings should provide value FQN ([#2151](#2151)) ([ad80044](ad80044)) * **policy:** key management crud ([#2110](#2110)) ([4c3d53d](4c3d53d)) * **policy:** Key management proto ([#2115](#2115)) ([561f853](561f853)) * **policy:** Modify get request to search for keys by kasid with keyid. ([#2147](#2147)) ([780d2e4](780d2e4)) * **policy:** Restrict KAS deletion when tied to Key ([#2144](#2144)) ([4c4ab13](4c4ab13)) * **policy:** Return KAS Key structure ([#2172](#2172)) ([7f97b99](7f97b99)) * **policy:** rotate keys rpc ([#2180](#2180)) ([0d00743](0d00743)) * **policy:** stored enhanced actions database migration, CRUD queries, SM updates ([#2040](#2040)) ([e6b7c79](e6b7c79)) * **sdk:** Add a KAS allowlist ([#2085](#2085)) ([d7cfdf3](d7cfdf3)) * **sdk:** add nanotdf plaintext policy ([#2182](#2182)) ([e5c56db](e5c56db)) * **sdk:** Use ConnectRPC in the go client ([#2200](#2200)) ([fc34ee6](fc34ee6)) ### Bug Fixes * **core:** access pdp cleanup before actions in ABAC decisioning ([#2123](#2123)) ([9b38a3c](9b38a3c)) * **core:** Autobump service ([#2080](#2080)) ([006c724](006c724)) * **core:** Autobump service ([#2104](#2104)) ([1f72cc7](1f72cc7)) * **core:** Autobump service ([#2108](#2108)) ([be5b7d7](be5b7d7)) * **core:** bump to go 1.24 and bump service proto module dependencies ([#2064](#2064)) ([94891a0](94891a0)) * **core:** Fix DPoP with grpc-gateway ([#2044](#2044)) ([4483ef2](4483ef2)) * **core:** fix service go.mod ([#2141](#2141)) ([3b98f6d](3b98f6d)) * **core:** Improves errors when under heavy load ([#2132](#2132)) ([4490a14](4490a14)) * **core:** Let legacy KAOs use new trust plugins ([#2218](#2218)) ([5aa6916](5aa6916)) * **core:** migrate from mitchellh/mapstructure to go-viper/mapstructure ([#2087](#2087)) ([0a3a82e](0a3a82e)) * **core:** update viper to 1.20.1 ([#2088](#2088)) ([09099e9](09099e9)) * **core:** Updates vulnerable dep go/x/net ([#2072](#2072)) ([11c02cd](11c02cd)) * **deps:** bump github.com/creasty/defaults from 1.7.0 to 1.8.0 in /service ([#2242](#2242)) ([86a9b46](86a9b46)) * **deps:** bump github.com/jackc/pgx/v5 from 5.5.5 to 5.7.5 in /service ([#2249](#2249)) ([d8f3b67](d8f3b67)) * **deps:** bump the internal group across 1 directory with 2 updates ([#2296](#2296)) ([7f92c70](7f92c70)) * **deps:** bump toolchain in /lib/fixtures and /examples to resolve CVE GO-2025-3563 ([#2061](#2061)) ([9c16843](9c16843)) * handle empty private and public key ctx structs ([#2272](#2272)) ([f3fc647](f3fc647)) * **policy:** remove predefined rules in actions protos ([#2069](#2069)) ([060f059](060f059)) * **policy:** return kas uri on keys for definition, namespace and values ([#2186](#2186)) ([6c55fb8](6c55fb8)) * update key_mode to provide more context ([#2226](#2226)) ([44d0805](44d0805)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>

jakedoublev added 3 commits May 21, 2025 12:24

pool config options for the db

bead794

config updates

a306740

propagate to example configs and docs

4355b6a

jakedoublev requested a review from a team as a code owner May 21, 2025 20:03

github-actions bot added external-contributor External Org Member docs Documentation size/m labels May 21, 2025

gemini-code-assist bot reviewed May 21, 2025

View reviewed changes

github-actions bot reviewed May 21, 2025

View reviewed changes

gemini-code-assist bot suggested changes May 21, 2025

View reviewed changes

service/pkg/db/db.go Outdated Show resolved Hide resolved

gemini suggestion

d78ece3

jakedoublev requested a review from Copilot May 21, 2025 20:10

Copilot AI reviewed May 21, 2025

View reviewed changes

service/pkg/db/db.go Show resolved Hide resolved

improve sensible defaults and non-zero config setting handling

5fcf355

strantalis reviewed May 21, 2025

View reviewed changes

service/pkg/db/db.go Outdated Show resolved Hide resolved

15 second connection timeout default:

3a84879

github-actions bot reviewed May 21, 2025

View reviewed changes

docs/Configuring.md Show resolved Hide resolved

jakedoublev requested a review from pflynn-virtru May 21, 2025 22:29

strantalis approved these changes May 22, 2025

View reviewed changes

jakedoublev added this pull request to the merge queue May 22, 2025

pflynn-virtru approved these changes May 22, 2025

View reviewed changes

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 22, 2025

jakedoublev added this pull request to the merge queue May 22, 2025

Merged via the queue into main with commit ed9ff59 May 22, 2025
29 checks passed

jakedoublev deleted the feat/db-conn branch May 22, 2025 16:25

opentdf-automation bot mentioned this pull request May 22, 2025

chore(main): release service 0.5.3 #2046

Merged

feat(core): enhance db configuration options #2285

feat(core): enhance db configuration options #2285

Uh oh!

Conversation

jakedoublev commented May 21, 2025

Proposed Changes

Checklist

Testing Instructions

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

Uh oh!

github-actions bot commented May 21, 2025

Benchmark Results:

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Error Summary:

Standard Benchmark Metrics Skipped or Failed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

github-actions bot commented May 21, 2025

Benchmark Results:

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Error Summary:

Standard Benchmark Metrics Skipped or Failed

Uh oh!

Uh oh!

github-actions bot commented May 21, 2025

Benchmark Results:

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Error Summary:

Standard Benchmark Metrics Skipped or Failed

Uh oh!

Uh oh!

github-actions bot commented May 21, 2025

Benchmark Results:

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Error Summary:

Standard Benchmark Metrics Skipped or Failed

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone