cheprasov / php-redis-client

RedisClient is a fast, fully-functional and user-friendly client for Redis, optimized for performance. RedisClient supports the latest versions of Redis starting from 2.6 to 6.0
MIT License
127 stars 41 forks source link

EmptyResponseException when executing flushdb() right after brpop(). #61

Closed Byter09 closed 6 years ago

Byter09 commented 6 years ago

So I'm writing a small library to wrap redis data types into PHP objects. Simple enough. But when testing the following:

public function testRpopTimeout(): void
{
    $list = new RedisList(self::$client, 'rpopTimeout');
    $this->expectException(EmptyResponseException::class);
    $list->rpop(1);
}

and then immediately cleaning up the database by using

public static function tearDownAfterClass()
{
    self::$client->flushdb();
}

I get the following error and stack trace:

PHP Fatal error:  Uncaught RedisClient\Exception\EmptyResponseException: Empty response. Please, check connection timeout. in /mnt/e/Projects/phpstorm/RedisObjects/vendor/cheprasov/php-redis-client/src/RedisClient/Protocol/RedisProtocol.php:111
Stack trace:
#0 /mnt/e/Projects/phpstorm/RedisObjects/vendor/cheprasov/php-redis-client/src/RedisClient/Protocol/RedisProtocol.php(164): RedisClient\Protocol\RedisProtocol->read()
#1 /mnt/e/Projects/phpstorm/RedisObjects/vendor/cheprasov/php-redis-client/src/RedisClient/Protocol/RedisProtocol.php(171): RedisClient\Protocol\RedisProtocol->sendRaw('*1\r\n$7\r\nFLUSHDB...')
#2 /mnt/e/Projects/phpstorm/RedisObjects/vendor/cheprasov/php-redis-client/src/RedisClient/Client/AbstractRedisClient.php(174): RedisClient\Protocol\RedisProtocol->send(Array)
#3 /mnt/e/Projects/phpstorm/RedisObjects/vendor/cheprasov/php-redis-client/src/RedisClient/Client/AbstractRedisClient.php(155): RedisClient\Client\AbstractRedisClient->executeProtocolCommand(Object(RedisClient\Protocol\RedisProtocol), Array, NULL) in /mnt/e/Projects/phpstorm/RedisObjects/vendor/cheprasov/php-redis-client/src/RedisClient/Protocol/RedisProtocol.php on line 111

I can't figure out whats going on.

Oh and yeah the command doesn't even reach Redis (this is the last one before the exception):

1518690040.542776 [8 127.0.0.1:52511] "BRPOP" "rpopTimeout" "1"

Any idea what might be the problem here?

Using Redis 4.0.1 but not RedisClient4x0, because I want this to be compatible with as many versions as possible.

Thanks. K. Becker

EDIT: I played around a bit and noticed this behaviour only when using blocking methods. For example, if I use brpop and after that test some other methods like

$list->rpush(1, 2, 3);
self::assertSame(1, $list->lpop());

lpop() does NOT pop 1 but instead null. Still figuring out why...

cheprasov commented 6 years ago

Hey @Byter09 I see, that you use BLPOP in your method RedisList:: brpop.

Please read very carefully about Redis command BLPOP - Blocking behavior Note:

If none of the specified keys exist, BLPOP blocks the connection until another client performs an LPUSH or RPUSH operation against one of the keys.

For first look. Your issue look like a trouble with blocking of connection.

Byter09 commented 6 years ago

There are two cases of what's happening.

First: Redis continues to block, even with a timeout. Weird but says so in the documentation. Thank you for showing me that.

Second: Assuming Redis blocks indefinitely, why does the test continue and not just wait right there? On my phone right now, so sorry for asking but are you blocking on the clients side or is Redis just returning (because of my timeout of 1) but still blocks the connection on the server side? But if that's the case, isn't that a bug?

EDIT: Additionally, any idea how to test this case or prevent it?

cheprasov commented 6 years ago

It does not look like a bug of Client or Redis, because the client has a test for BLPOP and it has a test for timeout too. Also, it was tested for all latest stable version of Redis: 2.6.17, 2.8.24, 3.0.7, 3.2.8, 4.0.8

Please see https://github.com/cheprasov/php-redis-client/blob/master/tests/Integration/Version2x6/ListsCommandsTest.php

But, I see that the RedisClient uses timeout' => 10 as config param in the test.

Also, you can try to check the behavior of Redis via redis-cli console

Byter09 commented 6 years ago

I'm back on my computer now and yeah I saw that too. Still, I'm calling

brpop($this-key, $timeout)

with

$timeout = 1

So shouldn't it return with null after a second?

See, this is the part I don't understand. I'm using the command as specified in the documentation.

EDIT: Also, your tests indicate the following:

$this->assertSame(null, $Redis->brpop(['list3', 'list4'], 1));

with a measured timeout of 1 <= timeout <= 2.5.

Which... is happening on my end too. (I can see phpunit pausing for a second) But null is not being returned. I'll test further and report all my results with their corresponding tests.

I hope I'm not totally misunderstanding something here. That'd be embarrasing :/

cheprasov commented 6 years ago

Try to use timeout = 10 for RedisClient, not for brpop command Example,

$Redis = ClientFactory::create([
    'timeout' => 10,
]);
Byter09 commented 6 years ago

Okay this is promising. Using your suggestion (and the one used in your tests apparently)

public static function setUpBeforeClass()
{
    self::$client = new RedisClient(['timeout' => 10]);
    self::$client->flushdb();
}

results in a correct assertion in this test:

public function testLpopTimeout(): void
{
    $list = new RedisList(self::$client, 'lpopTimeout');
    self::assertNull($list->lpop(1));
}

Now the question is: Why is that? Is the default timeout so small that it simply closes the connection (and raises an EmptyResponseException)? And is there a way to "fix" this by simply raising the timeout when using a blocking command to the timeout set in the method?

cheprasov commented 6 years ago

The default timeout is 2 sec, and it is too much for real production or highload servers. Anyway, you can use the config to configure RedisClient as you need :)

Byter09 commented 6 years ago

I know I can use the config to change the client as needed but I ment a "on-the-fly" change. Something like

$client->config('timeout', 10);

Also, if the default is 2 seconds, why does 10 make it any better? You said yourself that 2 is too much. Using the blocking command with a max. timeout of 1 second should make no difference with a client timeout of 2 or 10 seconds? I'm just curious. I want this to be highly efficient. And setting the client to 10 seconds simply to fix something does not fix the issue itself but instead make things slower in the worst case.