`JedisClusterKeyCommands ` makes extra `KEYSLOT` calls to the server

[upthewaterspout]

In JedisClusterKeyCommands, most of the commands find the node for a key using connection.getTopologyProvider().getTopology().getKeyServingMasterNode(key). However, there are a few commands like restore that call connection.clusterGetNodeForKey.

clusterGetNodeForKey actually ends up making a call to the redis server to execute the CLUSTER KEYSLOT command to determine the slot for the key. This is inefficient because it requires an extra round trip to the server to compute the slot of the key.

I can't see any particular reason why these commands are not calling getKeyServingMasterNode. That method computes the slot for the key on the client side, which is more efficient.

[mp911de]

Good catch. Do you want to submit a pull request to fix that issue?

[upthewaterspout]

Sure, I created a PR for this change - #2159.

I ran make test - which looks like it is running the unit test that exercises this code. I'm not quite sure what the best way to add new tests for this change is - or if that makes sense. I could add a unit test of JedisClusterKeyCommands that mocks the connection and asserts that clusterGetNodeForKey isn't called, but I'm not sure that has much value.