Skip to content

Increase pool reconnect + sleep#11139

Merged
abnegate merged 1 commit into1.8.xfrom
feat-pool-resilience
Jan 15, 2026
Merged

Increase pool reconnect + sleep#11139
abnegate merged 1 commit into1.8.xfrom
feat-pool-resilience

Conversation

@abnegate
Copy link
Copy Markdown
Member

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

The change modifies the pools registration closure in app/init/registers.php to retrieve two environment variables—_APP_CONNECTIONS_RECONNECT_ATTEMPTS and _APP_CONNECTIONS_RECONNECT_SLEEP—and apply them to a Group configuration. These environment variables are cast to integers and assigned via setReconnectAttempts and setReconnectSleep methods after all pool configurations are built.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description only contains the default contribution template with no actual content describing the changes, rationale, or test plan. Provide a meaningful description explaining what the PR does, why reconnect behavior configuration is needed, and how to verify the changes work.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly reflects the main change: adding environment variable configuration for pool reconnect attempts and sleep duration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
app/init/registers.php (1)

327-332: Parameterization looks good; consider validating input bounds.

The defaults (5 attempts, 2s sleep) are reasonable for reconnection resilience. However, the Utopia\Pools\Pool class has no input validation for these setters—they accept any integer value. If _APP_CONNECTIONS_RECONNECT_ATTEMPTS is set to 0 or negative, the reconnection loop in Pool::pop() will not execute, effectively disabling reconnection. Similarly, a negative sleep value could cause issues.

Consider adding bounds validation:

$reconnectAttempts = max(1, (int) System::getEnv('_APP_CONNECTIONS_RECONNECT_ATTEMPTS', 5));
$reconnectSleep = max(0, (int) System::getEnv('_APP_CONNECTIONS_RECONNECT_SLEEP', 2));

📜 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 61e886e and 7b940e3.

📒 Files selected for processing (1)
  • app/init/registers.php
⏰ 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). (3)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

@github-actions
Copy link
Copy Markdown

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link
Copy Markdown

✨ Benchmark results

  • Requests per second: 2,159
  • Requests with 200 status code: 388,685
  • P99 latency: 0.08214829

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 2,159 1,274
200 388,685 229,337
P99 0.08214829 0.160723984

@abnegate abnegate merged commit 5c922f8 into 1.8.x Jan 15, 2026
71 of 72 checks passed
@abnegate abnegate deleted the feat-pool-resilience branch January 15, 2026 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant