Repository navigation
fix(seaweedfs): configure postgres2 connection pool for the filer - #2906
Conversation
github-actions
Bot
added
area/storage
📝 WalkthroughWalkthroughSeaweedFS filer configuration is updated with PostgreSQL connection pool tuning parameters: ChangesSeaweedFS PostgreSQL Connection Pool Tuning
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! 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! This pull request addresses a performance bottleneck in the SeaweedFS filer configuration when using a PostgreSQL backend. By default, the Go database/sql pool was not maintaining idle connections, resulting in a new connection being established for every metadata lookup. The changes introduce explicit environment variables to manage the connection pool, ensuring connections are reused and significantly improving the responsiveness of S3 operations. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/system/seaweedfs/values.yaml`:
- Around line 52-54: Values WEED_POSTGRES2_CONNECTION_MAX_IDLE,
WEED_POSTGRES2_CONNECTION_MAX_OPEN, and
WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS are defined but the vendored Helm
chart under packages/system/seaweedfs/charts does not wire them into the
container environment; update the chart templates (the Deployment/StatefulSet
env section) to read these values and emit corresponding env vars (or map them
via value keys like postgresql/postgres2.* if the chart uses nested names),
e.g., add env entries that reference .Values.WEED_POSTGRES2_CONNECTION_MAX_IDLE,
.Values.WEED_POSTGRES2_CONNECTION_MAX_OPEN, and
.Values.WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS (or add passthrough
values in the chart’s values.yaml) so the SeaweedFS container actually receives
those WEED_POSTGRES2_* environment variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f0b807e-1ddb-49a0-8478-eb75acd181ae
📒 Files selected for processing (1)
packages/system/seaweedfs/values.yaml
| WEED_POSTGRES2_CONNECTION_MAX_IDLE: "20" | ||
| WEED_POSTGRES2_CONNECTION_MAX_OPEN: "40" | ||
| WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS: "600" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
SeaweedFS WEED_POSTGRES2_CONNECTION_MAX_IDLE WEED_POSTGRES2_CONNECTION_MAX_OPEN environment variables documentation
💡 Result:
In SeaweedFS, configuration options—including database connection pool settings—are typically defined in the filer configuration file (e.g., filer.toml) and can be overridden by environment variables [1]. For the PostgreSQL filer store, while standard documentation identifies variables like WEED_POSTGRES_CONNECTION_MAX_IDLE and WEED_POSTGRES_CONNECTION_MAX_OPEN [2], the specific variable names for the second PostgreSQL instance (often configured under a [postgres2] section in the configuration file) follow the same pattern by replacing the section prefix [3][4]. Based on the mapping logic used in SeaweedFS, the environment variables for the secondary PostgreSQL store are: WEED_POSTGRES2_CONNECTION_MAX_IDLE WEED_POSTGRES2_CONNECTION_MAX_OPEN These variables correspond to the configuration keys connection_max_idle and connection_max_open within the [postgres2] section of your configuration file. When set, they control the connection pool size for the secondary PostgreSQL database instance used by the SeaweedFS filer [3]. If you are setting these via environment variables, ensure the prefix matches the exact section name defined in your configuration file, as the system maps these by replacing the dot in configuration paths with an underscore [1]. You may also want to set WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS to manage the lifetime of these connections [5][3].
Citations:
- 1: https://deepwiki.com/seaweedfs/seaweedfs/3.1-command-line-interface
- 2: https://github.com/seaweedfs/seaweedfs/wiki/Environment-Variables
- 3: https://github.com/seaweedfs/seaweedfs/blob/b8236a10/weed/filer/postgres2/postgres2_store.go
- 4: https://github.com/seaweedfs/seaweedfs/blob/b8236a10/weed/filer/postgres/postgres_store.go
- 5: Optimize database connection pool settings for MySQL and PostgreSQL seaweedfs/seaweedfs#7645
🏁 Script executed:
#!/bin/bash
# Description: Search for Postgres2 connection pool environment variable usage in SeaweedFS chart and verify naming
# Check if the vendored chart references any Postgres2 connection pool settings
echo "=== Checking vendored chart for Postgres2 pool settings ==="
fd -t f . packages/system/seaweedfs/charts/ --exec rg -l 'POSTGRES2.*CONNECTION' {}
# Search for any existing Postgres2 environment variable patterns to confirm naming convention
echo -e "\n=== Existing WEED_POSTGRES2 environment variables in values.yaml ==="
rg -n 'WEED_POSTGRES2_' packages/system/seaweedfs/values.yamlRepository: cozystack/cozystack
Length of output: 571
SeaweedFS env var names match; Helm may not be wiring them
The SeaweedFS Postgres2 connection pool env var names used here (WEED_POSTGRES2_CONNECTION_MAX_IDLE, WEED_POSTGRES2_CONNECTION_MAX_OPEN, WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS) align with SeaweedFS’s [postgres2] configuration-to-environment mapping (replacing the section prefix with WEED_ + POSTGRES2_).
Main remaining risk: the vendored SeaweedFS Helm chart under packages/system/seaweedfs/charts/ didn’t reference any Postgres2 connection settings (no POSTGRES2.*CONNECTION hits), so these values may not actually be rendered into the SeaweedFS container environment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/system/seaweedfs/values.yaml` around lines 52 - 54, Values
WEED_POSTGRES2_CONNECTION_MAX_IDLE, WEED_POSTGRES2_CONNECTION_MAX_OPEN, and
WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS are defined but the vendored Helm
chart under packages/system/seaweedfs/charts does not wire them into the
container environment; update the chart templates (the Deployment/StatefulSet
env section) to read these values and emit corresponding env vars (or map them
via value keys like postgresql/postgres2.* if the chart uses nested names),
e.g., add env entries that reference .Values.WEED_POSTGRES2_CONNECTION_MAX_IDLE,
.Values.WEED_POSTGRES2_CONNECTION_MAX_OPEN, and
.Values.WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS (or add passthrough
values in the chart’s values.yaml) so the SeaweedFS container actually receives
those WEED_POSTGRES2_* environment variables.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures connection pool settings for SeaweedFS's PostgreSQL metadata store in packages/system/seaweedfs/values.yaml. Specifically, it sets WEED_POSTGRES2_CONNECTION_MAX_IDLE to 20, WEED_POSTGRES2_CONNECTION_MAX_OPEN to 40, and WEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS to 600. These settings prevent the Go SQL pool from keeping zero idle connections, which previously caused a fresh connection to be opened for every filer metadata lookup and introduced significant latency to S3 operations. I have no feedback to provide as there are no review comments to assess.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
dosubot
Bot
added
the
kind/backport
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
The change is correct and the env vars are wired: filer.extraEnvironmentVars flows through the seaweedfs.mergeExtraEnvironmentVars helper into the filer StatefulSet env block (filer-statefulset.yaml, lines 118-129), so WEED_POSTGRES2_CONNECTION_MAX_IDLE/OPEN/LIFETIME_SECONDS land in the container — the earlier bot comment about them not reaching the container does not hold. The WEED_<SECTION>_<KEY> naming matches how seaweedfs maps env vars to store config and mirrors the existing WEED_MYSQL_CONNECTION_MAX_* keys. Capping max_open at 40 against a two-replica filer default keeps total connections under the CNPG default of 100, and the inline comment usefully flags that constraint for anyone scaling filer.replicas. LGTM.
Non-blocking: a small helm-unittest asserting these keys appear in the filer env would lock in the behavior and pre-empt the "not wired" confusion, but it's not required to merge.
The vendored seaweedfs chart ships connection pool settings only for the mysql store, which cozystack does not use. The postgres2 store, which is enabled, has no pool settings, so the Go sql pool keeps zero idle connections and every filer metadata lookup opens a fresh PostgreSQL connection (TCP + TLS + SCRAM, ~300ms each). With ~5 path lookups per S3 operation this adds roughly 2 seconds of latency to every S3 request, regardless of cluster load. Measured on a production cluster (12 nodes, 2 filers): PUT of a 4KiB object went from 10s (p50) to 0.28s, HEAD from 2.6s to 0.16s after setting these values; pg_stat_activity confirmed connections were previously recycled every ~200ms and are now reused. max_open is capped at 40 so that two filer replicas stay below the CNPG default max_connections of 100. Signed-off-by: mattia-eleuteri <[email protected]>
a4c9a85 to
49cbea8
Compare
901bb98
into
cozystack:main
|
Successfully created backport PR for |
2 participants
What this PR does
WEED_POSTGRES2_CONNECTION_MAX_IDLE=20,WEED_POSTGRES2_CONNECTION_MAX_OPEN=40andWEED_POSTGRES2_CONNECTION_MAX_LIFETIME_SECONDS=600to the filerextraEnvironmentVarsinpackages/system/seaweedfs/values.yaml.The vendored seaweedfs chart only ships connection pool settings for the mysql store (
WEED_MYSQL_CONNECTION_MAX_*), which cozystack does not use. The postgres2 store, which cozystack enables, has no pool settings at all. As a result the Godatabase/sqlpool runs withSetMaxIdleConns(0): every filer metadata lookup opens a fresh PostgreSQL connection (TCP + TLS + SCRAM, ~300 ms each). With ~5 path-component lookups per S3 operation, this adds roughly 2 seconds of latency to every S3 request, even on an idle cluster.Measurements (production cluster, 12 nodes, 2 filers, CNPG backend)
Before / after applying these three variables, same test (sequential ops through the S3 gateway):
pg_stat_activityconfirmed the root cause: before the fix every backend was younger than ~200 ms (permanent connection churn); after the fix connections are reused for minutes. Real-traffic p95 ofSeaweedFS_filerStore_request_secondsdropped from 1.3–6.5 s to ~0.18 s.max_openis capped at 40 so that the default two filer replicas stay below the CNPG defaultmax_connectionsof 100.Release note
Summary by CodeRabbit