monospice / laravel-redis-sentinel-drivers

Redis Sentinel integration for Laravel and Lumen.
MIT License
101 stars 48 forks source link

Question: can it be used w/Horizon? #11

Closed pdbreen closed 6 years ago

pdbreen commented 6 years ago

Looks like a great package and I have plans to integrate & deploy early next week - provided it can be used w/Horizon (which requires/uses Redis to process queues and for all its tracking info). I'm assuming yes, that what's ultimately returned is / or implements that Laravel Redis interface, but thought I'd ask before diving in and hitting a dead end.

Any positive/negative results from others when using Horizon?

Thanks!

cyrossignol commented 6 years ago

Update: Versions 2.4.0+ provide support for Horizon.


Short answer: not...easily. I'm working on a patch now to make this seamless. The interfaces are fully-compatible, but Horizon uses some configuration values that need to be handled. We can set it up manually, but it's nicer if the package can do it for us :)

I hope to get this released sometime next week.

pdbreen commented 6 years ago

Hi - thanks for the feedback. Just getting back into this today. While the .env magic is nice, it seems using a config is pretty straight forward, esp. since I don't need to mix redis/redis-sentinel in my app - its either all one or the other.

Here's what I'm testing that seems to be working:

config/database.php

    'redis' => [

        // NEW - allow full driver swap via .env
        'driver' => env('REDIS_DRIVER', 'default'),

...

    // NEW SECTION, duplicates my redis connections within redis-sentinel
    'redis-sentinel' => [

        'default' => [
            [
                'host' => env('REDIS_HOST', 'localhost'),
                'port' => env('REDIS_PORT', 26379),
            ],
            'options' => [
                'service' => env('REDIS_SENTINEL_SERVICE', 'mymaster'),
                'parameters' => [
                    'password' => env('REDIS_PASSWORD', null),
                    'database' => 0,
                ],
            ],
        ],

        'horizon' => [
            [
                'host' => env('REDIS_HOST', 'localhost'),
                'port' => env('REDIS_PORT', 26379),
            ],
            'options' => [
                'service' => env('REDIS_SENTINEL_SERVICE', 'mymaster'),
                'parameters' => [
                    'password' => env('REDIS_PASSWORD', null),
                    'database' => 1,
                ],
            ],
        ],

    ],

.env

# For direct redis use
#REDIS_HOST=192.168.10.10
#REDIS_PORT=6379
#REDIS_DRIVER=default

# For redis-sentinel use
REDIS_HOST=localhost
REDIS_PORT=26379
REDIS_SENTINEL_SERVICE=beta
REDIS_DRIVER=redis-sentinel

I've only tested on a homestead VM so for, but should be able to roll out to an AWS hosted beta environment later today. I'll let you know if I come across any issues.

pdbreen commented 6 years ago

Biggest struggle deploying to multiple servers in AWS was getting the redis / redis sentinel / supervisord configs setup appropriately.

Once that was squared away and fail over confirmed working, it looks like the only remaining problem is the Horizon stats collector (run via the Laravel scheduler) is using a transaction which is causing the following exception. I'll see if I can turn up a solution.

Predis\NotSupportedException · Cannot initialize a MULTI/EXEC transaction over aggregate connections.
Raw
Predis\NotSupportedException Cannot initialize a MULTI/EXEC transaction over aggregate connections. 
    vendor/predis/predis/src/Transaction/MultiExec.php:70 Predis\Transaction\MultiExec::assertClient
    vendor/predis/predis/src/Transaction/MultiExec.php:50 Predis\Transaction\MultiExec::__construct
    vendor/predis/predis/src/Client.php:474 Predis\Client::createTransaction
    vendor/predis/predis/src/Client.php:396 Predis\Client::sharedContextFactory
    vendor/predis/predis/src/Client.php:461 Predis\Client::transaction
    vendor/laravel/framework/src/Illuminate/Redis/Connections/Connection.php:96 Illuminate\Redis\Connections\Connection::command
    vendor/laravel/framework/src/Illuminate/Redis/Connections/Connection.php:108 Illuminate\Redis\Connections\Connection::__call
    vendor/laravel/horizon/src/Repositories/RedisMetricsRepository.php:317 Laravel\Horizon\Repositories\RedisMetricsRepository::baseSnapshotData
    vendor/laravel/horizon/src/Repositories/RedisMetricsRepository.php:266 Laravel\Horizon\Repositories\RedisMetricsRepository::storeSnapshotForJob
    vendor/laravel/horizon/src/Repositories/RedisMetricsRepository.php:248 Laravel\Horizon\Repositories\RedisMetricsRepository::Laravel\Horizon\Repositories\{closure}
    vendor/laravel/framework/src/Illuminate/Support/Collection.php:341 Illuminate\Support\Collection::each
    vendor/laravel/horizon/src/Repositories/RedisMetricsRepository.php:249 Laravel\Horizon\Repositories\RedisMetricsRepository::snapshot
    vendor/laravel/horizon/src/Console/SnapshotCommand.php:33 Laravel\Horizon\Console\SnapshotCommand::handle
    [internal] call_user_func_array
    vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:29 Illuminate\Container\BoundMethod::Illuminate\Container\{closure}
    vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:87 Illuminate\Container\BoundMethod::callBoundMethod
    vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:31 Illuminate\Container\BoundMethod::call
    vendor/laravel/framework/src/Illuminate/Container/Container.php:549 Illuminate\Container\Container::call
    vendor/laravel/framework/src/Illuminate/Console/Command.php:180 Illuminate\Console\Command::execute
    vendor/symfony/console/Command/Command.php:264 Symfony\Component\Console\Command\Command::run
    vendor/laravel/framework/src/Illuminate/Console/Command.php:167 Illuminate\Console\Command::run
    vendor/symfony/console/Application.php:888 Symfony\Component\Console\Application::doRunCommand
    vendor/symfony/console/Application.php:224 Symfony\Component\Console\Application::doRun
    vendor/symfony/console/Application.php:125 Symfony\Component\Console\Application::run
    vendor/laravel/framework/src/Illuminate/Console/Application.php:88 Illuminate\Console\Application::run
    vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php:121 Illuminate\Foundation\Console\Kernel::handle
    artisan:37 

Referenced here: https://github.com/nrk/predis/issues/404

pdbreen commented 6 years ago

I've submitted some issues and corrective PR's to Horizon (https://github.com/laravel/horizon/issues/196, https://github.com/laravel/horizon/issues/193) and added a 'prefix' => 'horizon:' option to by sentinel horizon config, but I think I've now got it all running in my beta environment.

For the prefix, they are injecting that into the database config from the horizon config in a Horizon::use() method (https://github.com/laravel/horizon/blob/aab617e39d16b2ce86b7dc077b9d071edc17bcd5/src/Horizon.php#L86) - but this trickery doesn't work when the actual config isn't within database.redis scope, but database.redis-sentinel scope. Seems a bit of a hack on their part - I prefer the explicit setting in config.

Anyway, thanks for making this package available!

cyrossignol commented 6 years ago

@pdbreen, sorry for the (very lengthy) delay. I finally found some time to dig deeper into this. Not sure if you're still interested, or if you've moved on in your projects. The package now supports Horizon with version 2.4.0.

pdbreen commented 6 years ago

Hey @cyrossignol - thanks for the update. Yes still interested! I've been using my forked/patched version w/Horizon since Oct, but always nice to get back to an official release. I'll see if I can make the jump back in the next week or two. Thanks again