pulse icon indicating copy to clipboard operation
pulse copied to clipboard

redis cluster not supported for ingesting data

Open mihaileu opened this issue 11 months ago • 5 comments

Pulse Version

1.0.0-beta14

Laravel Version

10.47.0

PHP Version

8.3

Livewire Version

3.4.8

Database Driver & Version

No response

Description

Lalavel pulse doesn't accept redis cluster settings as ingest driver

{"message":"Laravel\\Pulse\\Support\\RedisAdapter::client(): Return value must be of type Redis|Predis\\Client|Predis\\Pipeline\\Pipeline, RedisCluster returned","context":{"exception":{"class":"TypeError","message":"Laravel\\Pulse\\Support\\RedisAdapter::client(): Return value must be of type Redis|Predis\\Client|Predis\\Pipeline\\Pipeline, RedisCluster returned","code":0,"file":"/var/www/genius/vendor/laravel/pulse/src/Support/RedisAdapter.php:144"}},"level":400,"level_name":"ERROR","channel":"production","datetime":"2024-03-11T12:06:12.701186+02:00","extra":{}}

Steps To Reproduce

set laravel with a redis cluster connection https://laravel.com/docs/10.x/redis#clusters

use redis for ingesting data in pulse

mihaileu avatar Mar 11 '24 10:03 mihaileu

Hi @mihaileu,

Are you using client-side sharding or native Redis clustering? I'm having trouble setting up either locally, so I'm figuring out where to focus my efforts.

jessarcher avatar Mar 13 '24 07:03 jessarcher

Redis clustering, using aws elasti cache redis cluster. These are the settings:

   'client' => env('REDIS_CLIENT', 'phpredis'),
 
    'options' => [
       'cluster' => env('REDIS_CLUSTER', 'redis'),
       'failover' => 2,
       'clusters' => [
           'default' => [
              [
               //connection details
              ]
           ] 
       ]
    ],

mihaileu avatar Mar 13 '24 08:03 mihaileu

Hi @mihaileu,

I've replicated the issue. The type error is easy enough to fix, however, I'm having trouble figuring out how to run the commands correctly and against which target node(s).

I'm also struggling to set up a cluster in GitHub Actions so any changes can be tested.

I've never used a Redis cluster before, so I'm in the dark here. Do you have any ideas what would be required, or can you submit a PR?

jessarcher avatar Mar 15 '24 05:03 jessarcher

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

github-actions[bot] avatar Mar 15 '24 05:03 github-actions[bot]

I got a cluster running in GitHub Actions so I've pushed my WIP at #335 to highlight the various errors.

It looks like Predis doesn't support stream commands like XADD with clusters, so that's probably a dead end.

PhpRedis looks a bit more promising, but there are two issues:

  1. There's an issue with pipelining. Not sure if this is a client limitation or something that can be solved.

  2. The stream commands expect a node key to be passed as the first argument, and I have no idea what that should be. The symptom of this is errors like this, where the command (e.g. XTRIM is being interpreted as the node key, which shuffles all the other arguments):

    Error running command [XTRIM laravel_database_laravel:pulse:ingest MINID ~ 946177445000]. Redis error: [ERR unknown command 'laravel_database_laravel:pulse:ingest', with args beginning with: 'MINID' '~' '946177445000' ].
    

jessarcher avatar Mar 15 '24 07:03 jessarcher

I'm seeing similar issues with a Laravel Vapor setup. Using the Redis 6.x Cluster. Temporarily using storage for PULSE_INGEST_DRIVER instead.

princejohnsantillan avatar May 29 '24 16:05 princejohnsantillan

Thanks all. For now, we're going to close this as there's no high-demand for this feature. We'll leave Jess' WIP PR open for now.

driesvints avatar Jul 04 '24 13:07 driesvints