[2.x] feat: phpredis auto-detect, cache:subscribe fix, settings memory cache#19
Merged
Conversation
If ext-redis is loaded, use phpredis as the driver. Otherwise fall back to Predis. No configuration change required by consumers. Documents persistent connections, Unix socket, and compression options in the README. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The cache subscriber hardcoded a Predis client check, causing it to immediately abort with "Expected Predis client" when phpredis (ext-redis) is the active driver. Route to a new `subscribePhpRedis()` method when the underlying client is a `\Redis` instance. phpredis `subscribe()` is blocking and callback-based rather than an iterator — set `OPT_READ_TIMEOUT = -1` for infinite timeout, and close the connection from within the callback to exit on `--once`. Predis path is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
phpredis 6.x accepts a Closure as the second argument to subscribe() at runtime, but the PHPStan stubs incorrectly type it as array|string. Add @phpstan-ignore-next-line to suppress the false positive. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…osure arg" This reverts commit 6cf88e0.
…edis fetches RedisCacheSettingsRepository::get() calls all() on every invocation, which calls cache->remember() every time — a Redis GET of the full flarum:settings blob on each call. On a typical forum page, settings are read 60–100 times. Each call fetches the entire settings blob from Redis (~154 KB on production), accounting for the majority of observed Redis egress bandwidth. Wrapping with MemoryCacheSettingsRepository restores the per-request in-process cache that Flarum core's SettingsServiceProvider provides by default. After the first settings read per request, all subsequent reads within the same request are local array lookups with zero network cost. Redis is hit at most once per request instead of once per settings->get() call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
1. Auto-detect phpredis, fall back to Predis
Detects the available Redis client at runtime rather than assuming Predis. If the
phpredisextension is loaded, it is used directly; otherwise falls back to Predis. No configuration change required.2. Fix
cache:subscribewith phpredisThe
cache:subscribecommand was hardcoded to checkinstanceof \Predis\Client, causing it to abort immediately when phpredis is active. Replaced with driver-agnostic routing:subscribe()withOPT_READ_TIMEOUT = -1and$redis->close()to exit on--onceread_write_timeout: 03. Add
MemoryCacheSettingsRepositorylayer to eliminate per-call Redis fetchesRedisCacheSettingsRepository::get()callsall()on every invocation, which callscache->remember()every time — a RedisGETof the fullflarum:settingsblob on each call.On a typical forum page, settings are read 60–100 times (core serializers, frontend JS variable injection, middleware, extensions). Each call fetches the entire settings blob from Redis — on production this is ~154 KB. That's up to ~15 MB of Redis egress per page load from settings reads alone, which at scale accounts for the majority of observed Redis outbound bandwidth (~300 Mbps on Nothing Community).
Flarum core's
SettingsServiceProviderusesMemoryCacheSettingsRepositoryto provide a per-request in-process cache.fof/redis'sSettingsprovider replaced the entire binding without restoring this layer.The fix wraps
RedisCacheSettingsRepositorywithMemoryCacheSettingsRepository, restoring the three-layer chain:After this change, Redis is hit at most once per request for settings instead of once per
settings->get()call. Expected Redis egress reduction: ~95%+ of settings-related bandwidth eliminated.