[PR #1248] [CLOSED] server: DB Connection Pool configuration #1207

New issue

Closed

opened 2026-02-27 09:11:19 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 09:11:19 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/lldap/lldap/pull/1248
Author: @thielj
Created: 8/20/2025
Status: ❌ Closed

Base: main ← Head: main

📄 Description

Alrighty, I tried my Rust hat with some help from AI. Go easy on me.

Automatic summary follows at the end.

Reference: Discussions

What still needs to be done is migrating database_url (or LLDAP_DATABASE_URL) from existing setups to the new db_options.url (LLDAP_DB_OPTIONS__URL). I tried to implement this but couldn't get my head around the mut stuff to promote the deprecated field to the new struct if the url field there is None. There's a // TODO comment somewhere this may fit. Your help here is very much appreciated.

The new configuration also affects the examples and in particular the default configuration file in docker that sets LLDAP_DATABASE_URL explicitly. I haven't touched these yet to keep the scope small.

I decided to increase max_connections default for SQLite. Async/await should benefit from this. If not, it shouldn't matter as the pool only creates one connection initially and everything else on demand. For non-SQLite databases, I defaulted this to min(5), max(10).

I've also defaulted the connection timeout to 3 seconds. This should help improve responsiveness if a remote DB isn't reachable.

Finally, I made the DB log level dependent on the verbose flag using either Debug or Info.

This pull request refactors how database connection options are configured and used in the server. Instead of relying on a single database_url field, it introduces a new DbOptions struct to group all database-related settings, making configuration more flexible and future-proof. The changes affect CLI argument parsing, configuration management, and database connection setup.

Configuration improvements:

Added a new DbOpts struct in server/src/cli.rs to handle database connection options via CLI, including URL, connection pool sizes, timeouts, and lifetimes. ([server/src/cli.rsR215-R242](diffhunk://#diff-7626d972d62203e5dc68698a3ac2adcef8ea1642b095870c93945da03324cfb6R215-R242))
Updated the main configuration struct (Configuration in server/src/configuration.rs) to use a new DbOptions field instead of a single database_url, with database_url now deprecated. ([server/src/configuration.rsL115-R142](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606L115-R142))
Implemented ConfigOverrider for DbOpts, allowing CLI/database environment overrides to be correctly applied to the configuration. ([[1]](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606R507-R529), [[2]](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606R479))
Added a warning message when the deprecated database_url field is used, guiding users to switch to db_options. ([server/src/configuration.rsR686-R690](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606R686-R690))

Database connection logic updates:

Refactored database connection setup in server/src/main.rs to use all options from DbOptions, including pool limits and timeouts, and made logging level configurable based on verbosity. ([server/src/main.rsL104-R145](diffhunk://#diff-01fd5002f7585d6f2b853a8c83e6c0a1acb2b16b6e6f8f581fb3365377d331bcL104-R145))
Updated function signatures and usages throughout the codebase to use DbOptions instead of just DatabaseUrl, including in setup_sql_tables and related server setup functions. ([[1]](diffhunk://#diff-01fd5002f7585d6f2b853a8c83e6c0a1acb2b16b6e6f8f581fb3365377d331bcL104-R145), [[2]](diffhunk://#diff-01fd5002f7585d6f2b853a8c83e6c0a1acb2b16b6e6f8f581fb3365377d331bcL278-R292))

These changes make database configuration more robust and extensible, and prepare for future enhancements or backend changes.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/lldap/lldap/pull/1248 **Author:** [@thielj](https://github.com/thielj) **Created:** 8/20/2025 **Status:** ❌ Closed **Base:** `main` ← **Head:** `main` --- ### 📄 Description Alrighty, I tried my Rust hat with some help from AI. Go easy on me. Automatic summary follows at the end. Reference: [Discussions](https://github.com/lldap/lldap/discussions/910) What still needs to be done is migrating database_url (or LLDAP_DATABASE_URL) from existing setups to the new db_options.url (LLDAP_DB_OPTIONS__URL). I tried to implement this but couldn't get my head around the `mut` stuff to promote the deprecated field to the new struct if the `url` field there is `None`. There's a `// TODO` comment somewhere this may fit. Your help here is very much appreciated. The new configuration also affects the examples and in particular the default configuration file in docker that sets LLDAP_DATABASE_URL explicitly. I haven't touched these yet to keep the scope small. I decided to increase max_connections default for SQLite. Async/await should benefit from this. If not, it shouldn't matter as the pool only creates one connection initially and everything else on demand. For non-SQLite databases, I defaulted this to min(5), max(10). I've also defaulted the connection timeout to 3 seconds. This should help improve responsiveness if a remote DB isn't reachable. Finally, I made the DB log level dependent on the verbose flag using either Debug or Info. --- This pull request refactors how database connection options are configured and used in the server. Instead of relying on a single `database_url` field, it introduces a new `DbOptions` struct to group all database-related settings, making configuration more flexible and future-proof. The changes affect CLI argument parsing, configuration management, and database connection setup. **Configuration improvements:** * Added a new `DbOpts` struct in `server/src/cli.rs` to handle database connection options via CLI, including URL, connection pool sizes, timeouts, and lifetimes. (`[server/src/cli.rsR215-R242](diffhunk://#diff-7626d972d62203e5dc68698a3ac2adcef8ea1642b095870c93945da03324cfb6R215-R242)`) * Updated the main configuration struct (`Configuration` in `server/src/configuration.rs`) to use a new `DbOptions` field instead of a single `database_url`, with `database_url` now deprecated. (`[server/src/configuration.rsL115-R142](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606L115-R142)`) * Implemented `ConfigOverrider` for `DbOpts`, allowing CLI/database environment overrides to be correctly applied to the configuration. (`[[1]](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606R507-R529)`, `[[2]](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606R479)`) * Added a warning message when the deprecated `database_url` field is used, guiding users to switch to `db_options`. (`[server/src/configuration.rsR686-R690](diffhunk://#diff-40bbcf477684ac3a8b76a2303a5f4e2e63214f4044f97cd1bef0eb4aad086606R686-R690)`) **Database connection logic updates:** * Refactored database connection setup in `server/src/main.rs` to use all options from `DbOptions`, including pool limits and timeouts, and made logging level configurable based on verbosity. (`[server/src/main.rsL104-R145](diffhunk://#diff-01fd5002f7585d6f2b853a8c83e6c0a1acb2b16b6e6f8f581fb3365377d331bcL104-R145)`) * Updated function signatures and usages throughout the codebase to use `DbOptions` instead of just `DatabaseUrl`, including in `setup_sql_tables` and related server setup functions. (`[[1]](diffhunk://#diff-01fd5002f7585d6f2b853a8c83e6c0a1acb2b16b6e6f8f581fb3365377d331bcL104-R145)`, `[[2]](diffhunk://#diff-01fd5002f7585d6f2b853a8c83e6c0a1acb2b16b6e6f8f581fb3365377d331bcL278-R292)`) These changes make database configuration more robust and extensible, and prepare for future enhancements or backend changes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>