Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.8] Delayed jobs are not taken to completion #29637

Closed
sera527 opened this issue Aug 19, 2019 · 2 comments
Closed

[5.8] Delayed jobs are not taken to completion #29637

sera527 opened this issue Aug 19, 2019 · 2 comments
Labels

Comments

@sera527
Copy link

sera527 commented Aug 19, 2019

  • Laravel Version: 5.8.31
  • PHP Version: 7.2.19
  • Database Driver & Version: Redis Cluster 5.0.5, php-extention phpredis 4.3.0

Description:

I use Redis Cluster with php-extention phpredis for queues. This worked fine until I added a function call delay().
I noticed that:

  • If I switch from cluster to single, the queue works.
  • If I switch from phpredis to predis/predis, the queue works.

Steps To Reproduce:

dispatch(
        (new SomeJob($data))
            ->delay(now()->addSeconds(30))
            ->onQueue('{some_queue}')
    );

database.php:

<?php

$redisConnStr = env('REDIS_CONNECTION', '127.0.0.1:6379');

$redis = [
    'client' => 'phpredis',
];

$redisConnections = explode(',', $redisConnStr);
if (count($redisConnections) === 1) {

    $params = explode(':', $redisConnections[0]);

    $redis = array_merge(
        $redis,
        [
            'cluster' => env('REDIS_CLUSTER', false),

            'default' => [
                'host'     => $params[0] ?? '127.0.0.1',
                'password' => env('REDIS_PASSWORD', null),
                'port'     => (int)($params[1] ?? 6379),
                'database' => env('REDIS_DATABASE', 0),
            ]
        ]
    );
}
else {
    $redisCacheCluster = [];

    foreach ($redisConnections as $conn){
        $params = explode(':', $conn);
        $redisCacheCluster[] = [
            'host'     => $params[0] ?? '127.0.0.1',
            'password' => env('REDIS_PASSWORD', null),
            'port'     => (int)($params[1] ?? 6379),
        ];
    }

    $redis             = array_merge(
        $redis,
        [
            'options'  => ['cluster' => 'redis'],
            'clusters' => [
                'default' => $redisCacheCluster,
                'cache' => $redisCacheCluster,
            ]
        ]
    );
}


return [
'default' => env('DB_CONNECTION', 'mysql'),
'connections' => [
'redis' => $redis,
];

.env:

QUEUE_CONNECTION=redis
REDIS_CONNECTION=ip:port,ip:port
REDIS_DATABASE=0

Run:

php artisan queue:work --queue={some_queue} --tries=1
@sera527
Copy link
Author

sera527 commented Aug 23, 2019

I found the location of the problem.
For dispatching job to Redis Cluster used method Illuminate\Redis\Connections\PhpRedisConnection::zadd().
The method execute this code:

return $this->command('rawCommand', $parameters);

The problem is that the interface of function rawCommand() is different for classes Redis and RedisCluster.
Redis:

/**
     * Send arbitrary things to the redis server.
     * @param   string      $command    Required command to send to the server.
     * @param   mixed       ...$arguments  Optional variable amount of arguments to send to the server.
     * @return  mixed
     * @example
     * <pre>
     * $redis->rawCommand('SET', 'key', 'value'); // bool(true)
     * $redis->rawCommand('GET", 'key'); // string(5) "value"
     * </pre>
     */
    public function rawCommand( $command, $arguments ) {}

RedisCluster:

/**
     * Send arbitrary things to the redis server at the specified node
     *
     * @param String | array $nodeParams key or [host,port]
     * @param String         $command    Required command to send to the server.
     * @param mixed          $arguments  Optional variable amount of arguments to send to the server.
     *
     * @return  mixed
     */
    public function rawCommand($nodeParams, $command, $arguments) { }

So I think that method Illuminate\Redis\Connections\PhpRedisConnection::zadd() (and maybe some others) must be overwritten in Illuminate\Redis\Connections\PhpRedisClusterConnection

@themsaid
Copy link
Member

themsaid commented Mar 8, 2020

fixed in #31838

@themsaid themsaid closed this as completed Mar 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants