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

reconnect after certain idle time and send AUTH on reconnect #18

Conversation

pityka
Copy link

@pityka pityka commented May 23, 2012

Hi, this solved my problems with a redis-server that A, had non-zero "timeout" config set B, requested password authentication.

@debasishg
Copy link
Owner

Thanks .. will have a look and pull over the weekend ..

@debasishg
Copy link
Owner

A question .. does this ignore the timeout specified in the redis conf file ?

@pityka
Copy link
Author

pityka commented May 24, 2012

I don't fully understand what you mean on "ignoring the timeout specified in the config".

What this does is quite simple: in case there was no sent messages to redis server after a certain idle time (set in connectionTimeout field of trait IO) , then on sending a new message the client reconnects before sending the new message.

So the client has to set the idletime to the same (or a bit smaller) value as the timeout specified in the redis server's config.

Without this patch using a redis server that has a timeout setup I got connection errors ("Connection dropped.." exception was thrown) after the redis server closed the connection.

Thanks

On 2012.05.24., at 10:19, Debasish Ghosh wrote:

A question .. does this ignore the timeout specified in the redis conf file ?

@debasishg
Copy link
Owner

The initial idea was to use the redis server's timeout for this same checking. If the redis server config timeout is defined to be 10s, then if the server does not receive any message within 10 secs it will close the connection. So, before every read and write from the client, I do a check if (!connected) connect; I don;t have to explicitly keep track of the time of inactivity since the server itself will close the connection on inactivity of 10s. But since you are still getting the error, I am not sure why if (!connected) connect; doesn't work.

My point is since we can track the closure of the connection from the server itself, why add another parameter for timeout ?

@pityka
Copy link
Author

pityka commented May 24, 2012

If you inspect the implementation of the connected : Boolean method, you find that it simple checks for socket != null . In case the server closed the connection you will still find that the socket is not null, you write to the socket and possibly got some exception out of the socket. As far as I know the redis server silently closes the connection (sends no message to the client about that event).

My point is since we can track the closure of the connection from the server itself, why add another parameter for timeout ?
I tried to do that. I could not reliable detect the fact from the client side that the server already closed the connection. (specifically: socketoutputstream threw exception on the second or third flush operation (but not the first) after the server closed the connection. That means I can not be sure that the flush succeeded or not. After the silently failed flush, the reading part of the client emits the "Connection dropped.." exception.)
I guess with some more extensive refactoring this should be possible.


Reply to this email directly or view it on GitHub:
#18 (comment)

@ghost
Copy link

ghost commented Jun 25, 2012

I'm interested in solving a possibly related issue: When using a client, one can get a RedisConnectionException. I believe this is due to the side effect of checking 'socket != null' as mentioned in the comment above. The reconnect check in write seems to pass through, but then on the readLine call, null is returned causing the exception at RedisProtocol.scala line 103 to occur. Any work towards solving this is greatly appreciated.

@ghost
Copy link

ghost commented Jun 25, 2012

Related to my previous post, I was able to workaround this by overriding the RedisClient's send method to attempt a well-defined number of retries should an exception occur. You may be able to come up with a more elegant solution, but this works as a stop-gap for the time being.

@debasishg
Copy link
Owner

@influenza - sounds neat. Do you have a pull request ?

@ghost
Copy link

ghost commented Jun 26, 2012

I'll put one together

@ghost
Copy link

ghost commented Jun 28, 2012

@debasishg Unfortunately sharing is blocked by process / legal issues at the moment. Basically, the two send methods should be wrapped in a while that wraps a try-catch block. Sorry I can't be more helpful :/

@debasishg
Copy link
Owner

Cool .. Let me try the implementation this weekend. Thanks for looking into it ..

debasishg added a commit that referenced this pull request Jun 30, 2012
@debasishg
Copy link
Owner

Implemented the retry semantics in RedisClient#send. Didn't do multiple retries - only a single retry on the exception RedisConnectionException. My test case works (the timeout test case in StringOperationsSpec, currently commented out since it needs a custom redis.conf)

@ghost
Copy link

ghost commented Jul 2, 2012

Yeah! Thanks mate

@debasishg
Copy link
Owner

closing this pull request ..

@debasishg debasishg closed this Jul 3, 2012
@timotta
Copy link

timotta commented Feb 6, 2014

I have this problem here too. Using RedisClientPool and an redis configured with auth, after some time i start receiving

java.lang.Exception: ERR operation not permitted
    at com.redis.Reply$$anonfun$6.applyOrElse(RedisProtocol.scala:94)
    at com.redis.Reply$$anonfun$6.applyOrElse(RedisProtocol.scala:93)
    at scala.runtime.AbstractPartialFunction.apply(AbstractPartialFunction.scala:33)
    at com.redis.Reply$$anonfun$4.applyOrElse(RedisProtocol.scala:63)
    at com.redis.Reply$$anonfun$4.applyOrElse(RedisProtocol.scala:63)
    at scala.PartialFunction$OrElse.apply(PartialFunction.scala:162)
    at com.redis.Reply$class.receive(RedisProtocol.scala:114)
    at com.redis.RedisClient.receive(RedisClient.scala:60)
    at com.redis.R$class.asBulk(RedisProtocol.scala:121)
    at com.redis.RedisClient.asBulk(RedisClient.scala:60)
    at com.redis.StringOperations$$anonfun$get$1.apply(StringOperations.scala:15)
    at com.redis.StringOperations$$anonfun$get$1.apply(StringOperations.scala:15)
    at com.redis.Redis$class.send(RedisClient.scala:21)
    at com.redis.RedisClient.send(RedisClient.scala:60)
    at com.redis.StringOperations$class.get(StringOperations.scala:15)
    at com.redis.RedisClient.get(RedisClient.scala:60)

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

Successfully merging this pull request may close these issues.

3 participants