scala-redis icon indicating copy to clipboard operation
scala-redis copied to clipboard

RedisCluster always tries to connect to localhost

Open brettcave opened this issue 11 years ago • 16 comments

When using redisCluster with a list of non-local node, rediscluster will still try and connect locally.

val redisCluster = new RedisCluster(Seq("192.168.0.1:6379"): _*) { val keyTag: Option[KeyTag] = Some(NoOpKeyTag) }

Running the above code without a localhost:6379 listener will fail:

java.lang.RuntimeException: java.net.ConnectException: Connection refused
    at com.redis.IO$class.connect(IO.scala:37) ~[redisclient_2.9.1-2.4.2.jar:na]
    at com.redis.RedisClient.connect(RedisClient.scala:41) ~[redisclient_2.9.1-2.4.2.jar:na]
    at com.redis.RedisClient.<init>(RedisClient.scala:44) ~[redisclient_2.9.1-2.4.2.jar:na]
    at com.redis.RedisClient.<init>(RedisClient.scala:46) ~[redisclient_2.9.1-2.4.2.jar:na]
    at com.redis.cluster.RedisCluster.<init>(RedisCluster.scala:59) ~[redisclient_2.9.1-2.4.2.jar:na]

Possibly because of this

def this() = this("localhost", 6379)

brettcave avatar Aug 22 '12 15:08 brettcave

Thanks for reporting .. will check up over the weekend ..

debasishg avatar Aug 22 '12 16:08 debasishg

Did the changes, pls check. Closing the issue. Will reopen if required.

debasishg avatar Aug 25 '12 16:08 debasishg

the changes don't allow pipelining to the cluster: redisCluster.pipeline { p=> // execute redis commands }

brettcave avatar Sep 03 '12 15:09 brettcave

ouch! Forgot to test pipelining .. will check very soon. Thanks for reporting ..

debasishg avatar Sep 03 '12 16:09 debasishg

Actually, what does pipelining to a cluster mean ? Select any node from the cluster and do the pipeline ? What do u think ?

debasishg avatar Sep 03 '12 16:09 debasishg

I am not sure if it should select any node, each command in the pipeline should use the same method for finding a node that the cluster would use for a normal command (i.e. each command should run on the node determined by nodeForKey(key) if valid).

e.g.

cluster.pipeline { p =>
    p.incr(key)
    p.expire(anotherKey)
}

p.incr would run on nodeForKey(key) and p.expire would run on nodeForKey(anotherKey) - would that not be the correct approach? (the equivalent of running cluster.incr(key) cluster.expire(anotherKey) and sending back the joined results.)

brettcave avatar Sep 13 '12 07:09 brettcave

On another note, we are using redis cluster for HA, not only for performance / scaling, so any write operation is run with cluster.onAllConns, while read operations are invoked directly, using nodeForKey for a single return. Would be great if this could be handled by the driver..

def faultTolerantRunOnAll[T](body: RedisClient => T): T = {
  checkAllHosts()
  try {
    redis.onAllConns { ac =>
      body(ac)
    }.head
  }
  catch {
    case err => {
      faultTolerantRunOnAll(body)
    }
  }
}

def checkAllHosts(): Boolean = {
  var curHost = ""
  var hasError: Boolean = false

  redis.clients.foreach { pool =>
    try {
      curHost = pool.toString()
      pool.withClient { client => }
    }
    catch {
      case ex: RuntimeException => {
        if (ex.getCause.isInstanceOf[ConnectException]) {
          hasError = true
          if (nodes.length > 1) {
            log.debug("Removing unconnectable redis node: " + curHost)
            deadNodes ++= List(curHost)
            nodes = nodes diff Seq(curHost)
            connect()
            checkAllHosts()
          }
          else throw new ConnectException("No connectable redis nodes in cluster")
        }
        else throw ex
      }
      case ex => throw ex
    }
  }
  hasError
}

brettcave avatar Sep 13 '12 07:09 brettcave

Regarding "any node for pipe-lining", I think all composing commands of a pipeline need to be executed on the same Redis server. The protocol is something like a transaction and the sequence is "MULTI" - [list of commands] - "EXEC" / "DISCARD". The server on getting a MULTI starts queuing responses - hence we need to fix on a server and then execute the whole pipeline there. Hence the thought of selecting a random node ..

debasishg avatar Sep 13 '12 08:09 debasishg

ah, true. But then this could potentially cause problems:

cluster.set(key, value) // sets on nodeX
cluster.pipeline { p =>
  p.get(key)  // gets from a random node, not necessarily nodeX
}

brettcave avatar Sep 13 '12 09:09 brettcave

good catch! I think pipelining doesn't look viable in a cluster implementation based on consistent hashing of keys ..

debasishg avatar Sep 13 '12 09:09 debasishg

regarding HA, your proposal looks good and possibly can be handled by the driver. Complications come up with failure handling .. if one of the nodes fail, do we consider the write successful ? Then comes up all the complexities of quorums and what not .. Keeping it simple, we can treat all-writes-or-none as the semantics. What do you think ?

debasishg avatar Sep 13 '12 09:09 debasishg

Unless it was possible to replicate the queuing on the driver side? so a server never gets a multi, it just gets the command, the response is returned to the driver, and then the driver queues up responses locally?

I have limited knowledge on the internals of redis, so not sure about how redis's queuing works, but is it not feasible to just concatenate the results from each command and return that?

HA: this is our own implementation that we use, and considering failure handling makes me wonder if it would be viable for a driver. One issue that we face is that if 1 redis node goes down, if we start it back up and restart the application then 1 node is out of sync with the rest, it may be missing keys. When we run this scenario, the redis nodes have no replication (due to the lack of master master replication currently), so this is a design with inherent consistency flaws

brettcave avatar Sep 13 '12 09:09 brettcave

Queuing on the driver side is tricky considering the fact that Redis does quite a few optimizations at the server level to improve performance. e.g. see http://redis.io/topics/transactions. Just queuing and concatenating may not work - need to ensure atomicity. In Redis server if the server crashes after executing a part of the pipelined commands, it detects during the subsequent start up and handles the erroneous condition.

Regarding HA and failover I initially thought of implementing something like https://github.com/ryanlecompte/redis_failover, but thought that I should wait till the sentinel stuff (http://redis.io/topics/sentinel) comes up. It is aimed at managing multiple Redis instances.

debasishg avatar Sep 13 '12 10:09 debasishg

yeah, we have looked at that, and was going to try implement something similar (redis_failover).

It does seem like pipelining in a cluster is not viable.

brettcave avatar Sep 13 '12 10:09 brettcave

Pipelining in a cluster is viable with the assumption of same-key-within-pipeline. Operations on same key (representing a single Redis structure) are guaranteed to hit same node - and they can form very useful transactions.

ponythewhite avatar Jan 06 '13 23:01 ponythewhite

Pipeling as we use it has the benefit in optimizing network throughput toi the DB system compared to single operation requests. considering the cluster machines will likely be installed in the same rack witin the same switch, networking between node is comparable more performant than from possible data producers (referring to the client side). Having dedicated queues for pipelining a single cluster node in my eyes is a misdesign - as you need to have knowledge of where data resides, what's the benefit of having a cluster (we've some server farms and I know rehashing because of new nodes is not a daily job - even if that benefit from the cluster is a nice to have)?

The complexity in the driver or client about getting metainformation, reacting on key-not-found-errors results in error prone software and bad maintenance. I'd rather have a two-hop-design within the cluster, so the cluster node itself is able to redirect to the correct node in case the client hit's the wrong node.

durchgedreht avatar May 24 '13 11:05 durchgedreht