[8.x] Add phpredis serialization and compression config support #40282
Conversation
TheLevti
force-pushed
the
feature/phpredis-options
branch
2 times, most recently
from
313f167
to
f7428d8
Compare
GrahamCampbell
changed the title
|
Does this play well with Horizon? |
TheLevti
force-pushed
the
feature/phpredis-options
branch
from
f7428d8
to
ec061ff
Compare
I have no idea as I never used horizon. This change just makes sure that if phpredis locking is used, we properly pack the passed arguments to eval and in addition adds config support for serialization and compression. |
TheLevti
force-pushed
the
feature/phpredis-options
branch
2 times, most recently
from
5e0b8d6
to
1bf86b6
Compare
TheLevti
force-pushed
the
feature/phpredis-options
branch
from
1bf86b6
to
88cb976
Compare
|
Does this have any breaking changes for existing applications, even minor or obscure breaking changes? |
The methods that were moved to the connection class are tagged as deprecated, so you can remove them later. The only breaking change I could think off is when someone had been using the Laravel's redis lock algorithm with a connection that has serialization and/or compression enabled, which just was not working before this pull request anyway (lock would never be released/acquired), so If we had such users of the framework, for them they would receive an exception revealing to them that they are using a serialization/compression algorithm that is not supported or that they are missing a required php extension to properly support redis lock with the given redis connection (in case of an old phpredis version). Before that they would silently not notice that locking is not working. |
|
Can you submit a PR to |
|
|
@TheLevti I tested this quickly with Laravel Horizon, but when adding jobs I get this error. When not using compression/serializer it works perfect... database.php |
can you show me more where you put this options? A bigger view of database.php. These options must be set per connection and not outside. Also it depends on how Horizon is using phpredis, what it does. |
|
`` `` |
|
Can you try this? 'redis' => [
'client' => env('REDIS_CLIENT', 'phpredis'),
'options' => [
'cluster' => env('REDIS_CLUSTER', 'redis'),
//'prefix' => env('REDIS_PREFIX', Str::slug(env('APP_NAME', 'laravel'), '_') . '_db_'),
],
'default' => [
'scheme' => env('REDIS_SCHEME', 'tcp'),
'url' => env('REDIS_URL'),
'host' => env('REDIS_HOST', '127.0.0.1'),
'password' => env('REDIS_PASSWORD', null),
'port' => env('REDIS_PORT', '6379'),
'database' => env('REDIS_DB', '0'),
'read_timeout' => 60,
'prefix' => 'd:',
'options' => [
'serializer' => Redis::SERIALIZER_MSGPACK,
'compression' => Redis::COMPRESSION_LZ4,
],
],
]; |
|
Same problem... |
Well I don't know what Horizon is doing, maybe same as the redlock algorithm it is using some eval calls, which need to be packed. Can you point me to some classes where it fails? It looks like Horizon is creating it's own connection by picking some params from the config: https://github.com/laravel/horizon/blob/5.x/src/Connectors/RedisConnector.php |
|
my error
aah yes that seems to be the "problem" |
Also https://github.com/laravel/horizon/blob/5.x/src/LuaScripts.php those script calls need to pack their arguments for commands where phpredis packs them. Some connection adjustments and additional tests with different serializer/compression setups should solve the issue. |
|
Thank you, finally |
|
This PR has been merged a long time ago. |
Are you maybe using an outdated phpredis extension? |
This is a follow up from #36791
This pull request allows the framework user to configure phpredis (https://github.com/phpredis/phpredis) serialization and compression options right in the config instead of the need to overwrite the service provider or a custom driver.
Supported serializers:
Supported compressors:
Also added tests for different compression algorithms.
Notes
My first attempt was to always serialize + compress the passed arguments to eval, but then later decided to let the user take care of this as we can not know what kind of lua script he wants to execute and whether all arguments are expected to be serialized/compressed. Instead I provided a helper method on the
PhpredisConnectionclass that the user may use. In addition I refactored the previous phpredis lock support (#36337) for these functionalities to use this new helper method.