feat(redis): add pool heartbeat and lifetime recycling#401
Conversation
Respect PoolOption heartbeat values on KeepaliveConnection by only registering a timer when the configured heartbeat is positive. Pass the raw heartbeat float to Timer::tick instead of truncating it through an integer helper, which avoids turning sub-second heartbeat intervals into near-busy-loop timers. Document that KeepaliveConnection max_idle_time eviction is driven by the heartbeat timer, clean up the stale inherited idle-close comment, and remove the completed Boost todo entry. Add regression coverage for disabled heartbeat creating no timer, for enabled heartbeat still closing idle connections, and update the existing heartbeat test to opt into a positive interval. Verification: ./vendor/bin/phpunit tests/Pool/HeartbeatConnectionTest.php; composer fix.
Add Redis connection generation timestamps and reuse-state tracking so pooled wrappers can distinguish active borrows from idle reusable connections. Centralize reconnect bookkeeping through markReconnected and make release failures return an invalid wrapper to the pool, preventing pool-slot leaks while forcing a clean reconnect on the next borrow.
Start an optional Redis pool heartbeat timer when configured and sweep only idle connections in the pool channel. Recycle expired idle generations, discard failed heartbeat checks, guard against late heartbeat completions after flushAll, and keep active borrowed connections untouched.
Validate pooled Redis wrappers before withConnection and withPinnedConnection callbacks run so expired or invalid idle generations reconnect before user code receives them. Keep Redis connection-only lifecycle helpers out of the proxy and facade command surfaces.
Expose optional Redis pool heartbeat_timeout and max_lifetime settings in the source database config while keeping heartbeat and lifetime recycling disabled by default. Update the Redis docs to describe checkout recycling, optional background heartbeat sweeps, heartbeat timeouts, idle recycling, and generation lifetime behavior.
Add Redis pool heartbeat coverage for disabled timers, idle and lifetime recycling, active-borrow safety, release reset failures, timeout cancellation, flush generation guards, proxy validation, and cluster master pings. Add a real Redis integration check for heartbeat pings and update existing Redis pool/proxy tests for the new lifecycle methods.
Remove the Redis pool heartbeat and max-lifetime item now that the feature, docs, config, and regression coverage have been added.
|
Warning Review limit reached
More reviews will be available in 24 minutes and 27 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a background heartbeat timer to ChangesRedis Pool Heartbeat & Lifetime Recycling
Sequence Diagram(s)sequenceDiagram
participant Timer
participant RedisPool
participant RedisConnection
participant Coroutine
participant Redis
rect rgba(100, 149, 237, 0.5)
note over Timer,RedisPool: Background heartbeat tick
Timer->>RedisPool: tick()
RedisPool->>RedisPool: heartbeat()
loop for each idle connection
RedisPool->>RedisConnection: isLifetimeExpired()?
alt expired
RedisPool->>RedisConnection: close()
else not expired
RedisPool->>RedisConnection: isIdleExpired()?
alt idle expired
RedisPool->>RedisConnection: close()
else healthy candidate
RedisPool->>RedisConnection: heartbeatCheck(timeout)
RedisConnection->>Coroutine: go(pingForHeartbeat)
Coroutine->>Redis: ping()
Redis-->>Coroutine: pong
Coroutine-->>RedisConnection: true
RedisConnection-->>RedisPool: true
RedisPool->>RedisPool: check generation stable?
alt generation unchanged
RedisPool->>RedisConnection: release()
else generation changed
RedisPool->>RedisConnection: close()
end
end
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/redis/src/RedisConnection.php`:
- Around line 684-689: When an exception occurs during the release process in
the catch block that catches Throwable, the $this->database property retains the
previously selected DB value, causing issues when the connection is later
reconnected. Fix this by resetting $this->database to its default value (null or
the configured default DB) inside the catch block before or alongside the
$this->markInvalid() call, ensuring the connection will select the correct
default DB on the next reconnection instead of using the stale DB value.
In `@tests/Pool/HeartbeatConnectionTest.php`:
- Line 53: In the test setup, the `$conn` variable assignment in the
setActiveConnection method call is never used after being assigned. Remove the
unused variable assignment by passing the anonymous class instance directly to
setActiveConnection without storing it in `$conn`, which will eliminate the
PHPMD warning while maintaining the same functionality.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 984a58ef-d416-4c3f-9c4d-90c711d2f1d1
📒 Files selected for processing (16)
src/boost/docs/redis.mdsrc/boost/todo.mdsrc/foundation/config/database.phpsrc/pool/src/KeepaliveConnection.phpsrc/redis/src/PhpRedisClusterConnection.phpsrc/redis/src/PhpRedisConnection.phpsrc/redis/src/Pool/RedisPool.phpsrc/redis/src/RedisConnection.phpsrc/redis/src/RedisProxy.phpsrc/support/src/Facades/Redis.phptests/Integration/Redis/RedisPoolHeartbeatIntegrationTest.phptests/Pool/Fixtures/KeepaliveConnectionStub.phptests/Pool/HeartbeatConnectionTest.phptests/Redis/RedisPoolHeartbeatTest.phptests/Redis/RedisPoolTest.phptests/Redis/RedisProxyTest.php
💤 Files with no reviewable changes (1)
- src/boost/todo.md
Greptile SummaryThis PR adds opt-in heartbeat sweeps and max-lifetime recycling to the Redis connection pool, fixes a capacity leak in
Confidence Score: 5/5Safe to merge. The new features are disabled by default and the capacity-leak fix improves correctness for all pools regardless of heartbeat configuration. The three-path eviction model is internally consistent: No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "refactor(redis): simplify release databa..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Reset the selected Redis database marker on every release path so a failed reset cannot leak a stale selected DB into the next reconnect. Add regression coverage for the marker reset, remove an unused heartbeat test variable, document the non-obvious request-idle heartbeat semantics, and broaden the max_lifetime jitter todo to cover Redis pools as well as database pools.
Remove the redundant selected database reset inside the release try block now that release normalizes the marker from a single finally block.
1 participant
Summary
withConnectionandwithPinnedConnectioncallbacks runThis PR also includes the prior local keepalive fix commit because it was already on the local
0.4branch and is part of the same pool heartbeat line of work.Notes
.env.examplefiles.Tests
./vendor/bin/phpunit --no-progress tests/Redis/RedisPoolHeartbeatTest.php./vendor/bin/phpunit --no-progress tests/Integration/Redis/RedisPoolHeartbeatIntegrationTest.php./vendor/bin/phpunit --no-progress tests/Redis/RedisProxyTest.php./vendor/bin/phpunit --no-progress tests/Redis/RedisConnectionTest.php./vendor/bin/phpunit --no-progress tests/Redis/RedisPoolTest.php./vendor/bin/phpunit --no-progress tests/Redis/PoolFactoryTest.phpcomposer fixSummary by CodeRabbit
Documentation
New Features
heartbeat,heartbeat_timeout, andmax_lifetimeenvironment variables.