Skip to content

Handle shared_buffers value when there are no memory requests#1743

Merged
dannyzaken merged 1 commit intonoobaa:masterfrom
dannyzaken:danny-fixes
Dec 3, 2025
Merged

Handle shared_buffers value when there are no memory requests#1743
dannyzaken merged 1 commit intonoobaa:masterfrom
dannyzaken:danny-fixes

Conversation

@dannyzaken
Copy link
Copy Markdown
Member

@dannyzaken dannyzaken commented Dec 3, 2025 •
edited by coderabbitai Bot
Loading

Explain the changes

  1. shared_buffers value for postgres is calculated based on memory requests.
  2. If memry requests are not set, we pass shared_buffers=0 to postgres, which is invalid
  3. If no memory requests are set for the DB, using shared_buffers=1G as a default value

Issues: Fixed #xxx / Gap #xxx

  1. https://issues.redhat.com/browse/DFBUGS-4746

Testing Instructions:

  • Doc added/updated
  • Tests added

Summary by CodeRabbit

  • Bug Fixes
    • Improved database configuration to apply a safe default buffer allocation when memory limits aren’t specified, ensuring more reliable startup and resource behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@dannyzaken dannyzaken requested a review from jackyalbo December 3, 2025 15:18
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 3, 2025 •
edited
Loading

Walkthrough

The change adds a nil-check in setPostgresConfig to handle cases where DBResources.Requests.Memory() returns nil by defaulting shared_buffers to 1GB; otherwise, it retains the previous calculation logic.

Changes

Cohort / File(s) Summary
Postgres Configuration Handling
pkg/system/db_reconciler.go		 Added nil-check for Requests.Memory() in setPostgresConfig with fallback to 1GB default for shared_buffers; preserves existing calculation when memory is specified

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single-file change with straightforward conditional logic
  • Default value assignment is non-controversial
  • Verify the 1GB default is appropriate for the system's constraints
  • Confirm the nil-check doesn't mask upstream issues with resource resolution

Suggested reviewers

  • liranmauda

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: handling shared_buffers when memory requests are absent.
Description check ✅ Passed The description covers the problem, solution, and references an issue, but Testing Instructions section lacks actual test steps and checkboxes remain unchecked.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d01ca0b and 97a3478.

📒 Files selected for processing (1)
  • pkg/system/db_reconciler.go (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: run-core-config-map-tests
  • GitHub Check: run-operator-tests
  • GitHub Check: run-kms-kmip-test
  • GitHub Check: run-kms-dev-test
  • GitHub Check: run-kms-key-rotate-test
  • GitHub Check: run-hac-test
  • GitHub Check: cnpg-deployment-test
  • GitHub Check: run-azure-vault-test
  • GitHub Check: run-cli-tests
  • GitHub Check: golangci-lint
  • GitHub Check: run-kms-tls-token-test
  • GitHub Check: run-kms-tls-sa-test
  • GitHub Check: run-admission-test

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot reviewed Dec 3, 2025
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be3b476 and d01ca0b.

📒 Files selected for processing (1)
  • pkg/system/db_reconciler.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/system/db_reconciler.go (1)
pkg/apis/noobaa/v1alpha1/noobaa_types.go (1)
  • NooBaa (41-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: run-core-config-map-tests
  • GitHub Check: cnpg-deployment-test
  • GitHub Check: run-cli-tests
  • GitHub Check: golangci-lint
  • GitHub Check: run-kms-kmip-test
  • GitHub Check: run-admission-test
  • GitHub Check: run-azure-vault-test
  • GitHub Check: run-kms-tls-sa-test
  • GitHub Check: run-kms-dev-test
  • GitHub Check: run-hac-test
  • GitHub Check: run-kms-key-rotate-test
  • GitHub Check: run-operator-tests
  • GitHub Check: run-kms-tls-token-test

Comment thread pkg/system/db_reconciler.go
@dannyzaken
handle shared_buffers value when there are no memory requests
97a3478 
Signed-off-by: Danny Zaken <dannyzaken@gmail.com>
@dannyzaken dannyzaken merged commit 0bc8b7e into noobaa:master Dec 3, 2025
13 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@nimrod-becker nimrod-becker nimrod-becker approved these changes

@jackyalbo jackyalbo jackyalbo approved these changes

Assignees

No one assigned

Labels

size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dannyzaken @nimrod-becker @jackyalbo