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

Fatal when transaction broken in Sentinel #459

Closed
gr1ev0us opened this issue Aug 28, 2017 · 2 comments
Closed

Fatal when transaction broken in Sentinel #459

gr1ev0us opened this issue Aug 28, 2017 · 2 comments

Comments

@gr1ev0us
Copy link

Problem
In persistent mode when transaction was aborted I get fatal error
\Predis\Connection\Aggregate\SentinelReplication::assertConnectionRole:526
Cannot use object of type Predis\Response\Status as array in ../src/Connection/Aggregate/SentinelReplication.php on line 526

How to reproduce
Create script, which doesn't close transaction

$config = [
    'parameters' => [
        'tcp://127.0.0.1:26379',
    ],
    'options' => [
        'replication' => 'sentinel',
        'service' => 'test',
        'parameters' => [
            'password' => 'test',
            'timeout' => 0.15,
            'persistent' => true, 
        ],
    ],
    'retryWait' => 250,
];

$predis = new \Predis\Client($config['parameters'], $config['options']);

if ($predis->getConnection() instanceof \Predis\Connection\Aggregate\SentinelReplication) {
    $predis->getConnection()->setRetryWait(250);
}

$predis->multi();
echo 'Set the value';
$predis->set('key', 'test');
throw new Exception('Some error');

Then i am starting the server:

$ php -S 127.1:5000

And when we request our script, predis will produce fatal error.

How to solve
We need to check $actualRole var for object or array.

@schematical
Copy link

Okay so for anyone in the future that sees this what happened to me when I got Cannot use object of type Predis\Response\Status as array was that I had a multi() transaction open to do a write but before I closed that transaction I needed to do a get to get info but it blew up because the multi had not finished.

To fix it I had to execute the multi transaction then run the get then start another multi transaction. Seems like a flaw in Predis but its easy enough to work around. Not super efficient, but easy to work around.

@nrk
Copy link
Contributor

nrk commented Aug 29, 2020

The problem actually is that you are using persistent connections (see 'persistent' => true in your default parameters) when persistent connections should be handled with care as they can lead exactly to these kinds of issues.

On the server-side Redis keeps tabs of the current state of each MULTI ... EXEC transaction on a per-client basis based on the state of the connection with the client so closing the connection before an EXEC or DISCARD effectively clears the queued commands automatically and nothing is left pending.

What happens when using persistent connections in your script is that when the script exits (either intentionally or because of a fatal error) leaving a pending MULTI open with no explicit EXEC or DISCARD command, the underlying stream socket resource to Redis is still held open by PHP so Redis thinks there's still an ongoing transaction associated to that connection (as described in the previous paragraph). Then PHP assigns that persistent stream socket resource to the next script and boom, instead of the response to a normal SET operation you get a +QUEUED response as if you where inside a transaction and well indeed you are still inside a MULTI ... EXEC block at the connection level.

If PHP exposed a way to assign some kind of custom state to persistent resources in userland code then there could be a way to handle this directly inside of Predis but unfortunately this is not the case.

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

No branches or pull requests

3 participants