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

Automatic reconnect behavior inconsistent #1757

Open
2 tasks done
johannes85 opened this issue May 13, 2020 · 12 comments
Open
2 tasks done

Automatic reconnect behavior inconsistent #1757

johannes85 opened this issue May 13, 2020 · 12 comments
Assignees

Comments

@johannes85
Copy link

johannes85 commented May 13, 2020

Expected behaviour

When automatically reconnect the auth command should always be send.

Actual behaviour

When the server wents away the reconnect sends the auth command, but when reconnecting after calling the close method the auth command isn't sent.

I'm seeing this behaviour on

  • OS: Debian Jessie
  • Redis: 6.0.1
  • PHP: 7.2.30
  • phpredis: 5.2.2

Steps to reproduce, backtrace or example script

Works:

$client = new \Redis();
$client->connect('127.0.0.1', 6379);
$client->auth('foo');
$client->set('foo', 'bar');
sleep(10);  // Restarting redis server while script is waiting
$client->set('foo', 'bar');

Isn't working, produces PHP Fatal error: Uncaught RedisException: NOAUTH Authentication required.

$client = new \Redis();
$client->connect('127.0.0.1', 6379);
$client->auth('foo');
$client->set('foo', 'bar');
$client->close();
$client->set('foo', 'bar');

I've checked

  • There is no similar issue from other users
  • Issue isn't fixed in develop branch
@Venorcis
Copy link

I have the same problem, though in a long running script on a replica where (apparently) a connection reset sometimes occurs. After this happened I get the same 'NOAUTH Authentication required' error.

@xiaoliwang
Copy link

I have the same problem. Only if I catch the exception and close the connection, the error 'NOAUTH AUTHENTICATION required' disappear.

@michael-grunder michael-grunder self-assigned this Jul 21, 2020
@michael-grunder
Copy link
Member

I can't replicate this with PhpRedis 5.3.0. Both of the examples work fine (and reauthorize the connection)

@xiaoliwang
Copy link

class RedisBase extends Redis	
	public function __construct()
    {
        $this->connect('127.0.0.1', 6379);
        $this->auth('password');
    }
}

new RedisBase();

When i use tcpkill to kill the tcp connection, I get the 'NOAUTH Authentication required' throwed by connect function.

@michael-grunder
Copy link
Member

Can you provide a bit more context?

From the above sample code, it seems like you'd have a difficult time timing the tcpkill call. Or do you mean you're calling it in a loop?

@xiaoliwang
Copy link

Well. I use php 7.3.6 And php-redis 5.3.0
Use nginx and php-fpm

I use the script as below, and I can get the right value.

<?php
$redis = new Redis();
$redis->pconnect('127.0.0.1');
$redis->auth('password');
echo $redis->get('a');

After that, I use tcpkill to kill the connection. And then stop tcpkill running.
Then I visit the page, I will get the error below.

Stack trace:
#0 /home/www/missevan-web/public/index_temp.php(3): Redis->pconnect('r-bp1bts3fbiqme...')
#1 {main}
thrown in /home/www/missevan-web/public/index_temp.php on line 3
NOTICE: PHP message: PHP Fatal error: Uncaught RedisException: NOAUTH Authentication required. in /home/www/missevan-web/public/index_temp.php:3

And then I do the samething to the script below. It's OK. No error appear.

<?php
$redis = new Redis();
try{
  $redis->pconnect('127.0.0.1');
  $redis->auth('password');
  echo $redis->get('a');
} catch(Exception $e) {
  $redis->close();
}

@michael-grunder
Copy link
Member

Ok I will try that setup tomorrow morning.

@michael-grunder
Copy link
Member

I can replicate this. I'll post an update when I figure out exactly what's going on.

@michael-grunder
Copy link
Member

michael-grunder commented Jul 23, 2020

OK, this exception is being thrown in our ECHO "liveness check".

There are two workarounds (as long as you're running >= 5.3.0):

  1. Change your pconnect call to include the AUTH information:
$redis->pconnect('127.0.0.1', 9999, 1, NULL, 0, 0, ['auth' => ['password']]);
  1. Disable the redis.pconnect.echo_check_liveness INI setting. I don't recommend this if you've ever had persistent connections get into a bad state. If this has never happened to you then it's probably OK.

@JimmyBanks
Copy link

Thanks @michael-grunder

Will this essentially be a necessity to fix the issue for persistent connections, and the best practice moving forward? Or is the underlying issue with the auth() method also fixable in a future version?

@michael-grunder
Copy link
Member

Once persistent connections are fixed we won't need the liveness check at all, so the problem will go away.

That said, this specific issue is fixable before that, because in actuality a NOAUTH error is sufficient in and of itself to confirm "liveness". The whole point of the liveness check is to make sure there aren't unconsumed replies on the connection when we pull it from the persistent connection pool. If Redis tells us NOAUTH we know for a fact there aren't any replies since we're not even logged in.

I think it will be safe to add NOAUTH as a "success" for the purposes of the liveness check, which should also be pretty simple to implement.

@xiaoliwang
Copy link

Thanks @michael-grunder 。I will try。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants