-
Notifications
You must be signed in to change notification settings - Fork 218
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
re-authenticate and re-select database when disconnect #101
base: master
Are you sure you want to change the base?
Conversation
Thanks a lot .. will do a review and merge over the weekend. |
hi, any news on that? |
Apologies for the delay .. was awfully busy with day job. Will merge the PR this weekend for sure .. |
I was trying out the changes and getting a StackOverFlowError .. Welcome to Scala version 2.10.2 (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_65).
Type in expressions to have them evaluated.
Type :help for more information.
scala> import com.redis._
import com.redis._
scala> val p = new RedisClientPool("localhost", 6379, secret=Some("foobared"), maxIdle=1)
p: com.redis.RedisClientPool = localhost:6379
scala> p.withClient { client => client.set("x","y") }
log4j:WARN No appenders could be found for logger (com.redis.RedisClient).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
java.lang.RuntimeException: java.lang.StackOverflowError
at com.redis.IO$class.connect(IO.scala:36)
at com.redis.RedisClient.connect(RedisClient.scala:89)
at com.redis.RedisCommandOperations$class.initialize(RedisClient.scala:60)
at com.redis.RedisClient.initialize(RedisClient.scala:89)
at com.redis.Redis$class.reconnect(RedisClient.scala:49)
at com.redis.RedisClient.reconnect(RedisClient.scala:89)
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$3.applyOrElse(RedisProtocol.scala:58)
at com.redis.Reply$$anonfun$3.applyOrElse(RedisProtocol.scala:58)
at scala.PartialFunction$OrElse.apply(PartialFunction.scala:162)
at com.redis.Reply$$anonfun$1.applyOrElse(RedisProtocol.scala:48)
at com.redis.Reply$$anonfun$1.applyOrElse(RedisProtocol.scala:48)
at scala.PartialFunction$OrElse.apply(PartialFunction.scala:162)
at com.redis.Reply$class.receive(RedisProtocol.scala:114) |
Hi, Im going to see what is happening until monday. []'s Tiago Albineli Motta On Sat, Feb 22, 2014 at 1:21 PM, Debasish Ghosh [email protected]:
|
cool .. I will merge it once I hear from you .. |
The problem happens when a redis-server is not configured to have password In this redis-server returns an error, and we try to reconnect. The So now, im changing auth to use another send command that only reconnects, Tiago Albineli Motta On Sat, Feb 22, 2014 at 1:40 PM, Debasish Ghosh [email protected]:
|
+1 .. will wait for the new PR .. |
@debasishg i anexed to this pull request the other commit. It's ok for you? Without password on redis-server:
With password on redis-server:
|
Thanks for the fix. I did apply it and verified that it works. Then I noticed some code duplication between |
Great. The only problem is that it looks like now the "initialize" method only selectDatabase when there is a secret defined. It is not the correct behavior. Maybe we show let this part like before:
|
Ok .. made that change in branch auth-fix .. now ready for the final release. |
Do you support reconnect and recover subscriptions ? |
No, it's not supported as of now. I need to work on this PR and check if this can make to master. Horribly out of time these days. I will try over the coming weekend. |
When a client on a connection pool looses its connection to redis-server, is necessary to re-authenticate this client. The problem can be reproduced following these steps:
The change in this commit correct this problem, re-authenticanting and re-selecting the redis database when connection was lost. Also, it includes a feature allowing single client to have database and authentication: