feature/separate-wal-storage #605
Conversation
dosubot
bot
added
the
size:M
WalkthroughThis pull request introduces a consistent configuration for Write-Ahead Logging (WAL) storage across multiple PostgreSQL deployments. The changes involve adding a new Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/system/keycloak/templates/db.yaml (1)
9-10: Parameterize the new WAL storage size.The addition of a dedicated
walStoragesection with a hard-codedsize: 1Giis correct and beneficial for performance. However, consider allowing this value to be overridden via Helm values if different environments have diverse storage requirements.packages/system/seaweedfs/templates/database.yaml (1)
10-11: Align WAL storage size with environment-specific settings.Adding a separate WAL storage is a sound approach for production reliability. Similar to the Keycloak configuration, consider making this size configurable in order to support environments that might need more or less capacity.
packages/extra/monitoring/templates/grafana/db.yaml (1)
9-10: Unify WAL storage sizing with existing Helm values.While
storage.sizeis controlled by.Values.grafana.db.size, the WAL storage is defined as a fixed1Gi. For consistency, consider introducing a.Values.grafana.db.walSizeparameter to align these configurations and allow flexible sizing across different deployments.packages/extra/monitoring/templates/alerta/alerta-db.yaml (1)
13-14: Consider parameterizing the WAL storage sizeWhile adding WAL storage configuration is good, consider making the size configurable through values for consistency with other storage parameters.
walStorage: - size: 1Gi + size: {{ .Values.alerta.walSize | default "1Gi" }}packages/apps/ferretdb/templates/postgres.yaml (1)
19-20: Align WAL storage configuration with main postgres templateFor consistency with the main postgres template, consider parameterizing the WAL size through values.
walStorage: - size: 1Gi + size: {{ .Values.walSize | default "1Gi" }}packages/apps/postgres/values.yaml (1)
10-10: Add documentation for the walSize parameterPlease add parameter documentation following the existing style:
## @param size Persistent Volume size ## @param replicas Number of Postgres replicas ## @param storageClass StorageClass used to store the data +## @param walSize Write-Ahead Log storage size ## external: false size: 10Gi walSize: 1Gi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/apps/ferretdb/templates/postgres.yaml(1 hunks)packages/apps/postgres/Chart.yaml(1 hunks)packages/apps/postgres/templates/db.yaml(1 hunks)packages/apps/postgres/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)packages/core/installer/values.yaml(1 hunks)packages/extra/monitoring/templates/alerta/alerta-db.yaml(1 hunks)packages/extra/monitoring/templates/grafana/db.yaml(1 hunks)packages/system/keycloak/templates/db.yaml(1 hunks)packages/system/seaweedfs/templates/database.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/apps/postgres/Chart.yaml
- packages/core/installer/values.yaml
🔇 Additional comments (2)
packages/apps/versions_map (1)
70-71: Confirm the version references.Switching from a HEAD pointer to a specific commit (
cb7b8158) forpostgres 0.8.0and adding a new version entry0.8.1 HEADhelps differentiate stable vs. development versions. Ensure that the new references are properly tested and that any code referencingpostgres 0.8.0or0.8.1is updated accordingly.packages/apps/postgres/templates/db.yaml (1)
26-27: LGTM! Well-structured WAL storage configurationThe implementation correctly uses templating with a default value, allowing for customization while maintaining consistency.
| @@ -7,6 +7,7 @@ | |||
| ## | |||
| external: false | |||
| size: 10Gi | |||
| walSize: 1Gi | |||
There was a problem hiding this comment.
The name walSize may be confusing with max_wal_size, min_wal_size in postgresql.conf.
I'd go for walStorageSize here.
| storage: | ||
| size: {{ required ".Values.size is required" .Values.size }} | ||
| {{- with .Values.storageClass }} | ||
| storageClass: {{ . }} | ||
| {{- end }} | ||
|
|
||
| walStorage: | ||
| size: 1Gi |
There was a problem hiding this comment.
But default value of max_wal_size is also 1GB. This means postgresql will hit out-of-space on first full WAL file that wasn't closed by the checkpoint that occurred before the file size limit.
Max WAL file size needs to be decreased for proper operation. I'd suggest 256MB. This will allow having at least two full WALs, and also preserve some room for replica startup operations.
Ex:
postgresql:
parameters:
max_wal_size: "256MB"
This is applicable here and in other places where walStorage.size was introduced.
Summary by CodeRabbit
Release Notes
New Features
Version Updates
Infrastructure Changes