{"action":"created","issue":{"url":"https://api.github.com/repos/xetorthio/jedis/issues/1877","repository_url":"https://api.github.com/repos/xetorthio/jedis","labels_url":"https://api.github.com/repos/xetorthio/jedis/issues/1877/labels{/name}","comments_url":"https://api.github.com/repos/xetorthio/jedis/issues/1877/comments","events_url":"https://api.github.com/repos/xetorthio/jedis/issues/1877/events","html_url":"https://github.com/xetorthio/jedis/issues/1877","id":370222789,"node_id":"MDU6SXNzdWUzNzAyMjI3ODk=","number":1877,"title":"There is a bug in the renewClusterSlots method in the Jedis cluster function","user":{"login":"lixyou","id":29372771,"node_id":"MDQ6VXNlcjI5MzcyNzcx","avatar_url":"https://avatars3.githubusercontent.com/u/29372771?v=4","gravatar_id":"","url":"https://api.github.com/users/lixyou","html_url":"https://github.com/lixyou","followers_url":"https://api.github.com/users/lixyou/followers","following_url":"https://api.github.com/users/lixyou/following{/other_user}","gists_url":"https://api.github.com/users/lixyou/gists{/gist_id}","starred_url":"https://api.github.com/users/lixyou/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lixyou/subscriptions","organizations_url":"https://api.github.com/users/lixyou/orgs","repos_url":"https://api.github.com/users/lixyou/repos","events_url":"https://api.github.com/users/lixyou/events{/privacy}","received_events_url":"https://api.github.com/users/lixyou/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-10-15T15:38:21Z","updated_at":"2018-10-15T16:11:12Z","closed_at":null,"author_association":"NONE","body":"### Expected behavior\r\n\r\nJedis is able recache slots when a JedisRedirectionException occurs.\r\n\r\n### Actual behavior\r\n\r\nJedis's implemention is as follows:\r\n\r\n```\r\npublic void renewClusterSlots(Jedis jedis) {\r\n  //If rediscovering is already in process - no need to start one more same rediscovering, just return\r\n  if (!rediscovering) {\r\n    try {\r\n      w.lock();\r\n      rediscovering = true;\r\n      if (jedis != null) {\r\n        try {\r\n          discoverClusterSlots(jedis);\r\n          return;\r\n        } catch (JedisException e) {\r\n          //try nodes from all pools\r\n        }\r\n      }\r\n      for (JedisPool jp : getShuffledNodesPool()) {\r\n        Jedis j = null;\r\n        try {\r\n          j = jp.getResource();\r\n          discoverClusterSlots(j);\r\n          System.out.println(j.getClient().getHost() + \":\" + j.getClient().getPort());\r\n\r\n          // Note: Don't use return here!\r\n          return;\r\n        } catch (JedisConnectionException e) {\r\n          e.printStackTrace();\r\n          // try next nodes\r\n        } finally {\r\n          if (j != null) {\r\n            j.close();\r\n          }\r\n        }\r\n      }\r\n    } finally {\r\n      rediscovering = false;\r\n      w.unlock();\r\n    }\r\n  }\r\n}\r\n```\r\nUse the for loop to traverse the JedisPool, but use return in the for loop, resulting in a direct return, not all the slots are cached. Please look at  the note above.\r\n\r\n### Steps to reproduce:\r\n\r\n1. Use '127.0.0.1:7000, 127.0.0.1:7001, 127.0.0.1:7002' build redis cluster. All three machine are master.\r\neg: \r\n\r\n```\r\n>redis-3.2.4/src/redis-trib.rb check  127.0.0.1:7000\r\n>>> Performing Cluster Check (using node 127.0.0.1:7000)\r\nM: 22c26c1565a89d97c21437ced30fefa0f0d20b7b 127.0.0.1:7000\r\n   slots:0-5460 (5461 slots) master\r\n   0 additional replica(s)\r\nM: 63aed469c611404cd3ccf4a5841b449fb14f7676 127.0.0.1:7001\r\n   slots:5461-10922 (5462 slots) master\r\n   0 additional replica(s)\r\nM: ee749e3425646d8c8ae79a470e9595f78cebb906 127.0.0.1:7002\r\n   slots:10923-16383 (5461 slots) master\r\n   0 additional replica(s)\r\n[OK] All nodes agree about slots configuration.\r\n>>> Check for open slots...\r\n>>> Check slots coverage...\r\n[OK] All 16384 slots covered.\r\n```\r\n\r\n2. Write test cases, the slot of the key for 'cluster1' is 12539, this slot is at the node of 7002.  \r\n```\r\nSet<HostAndPort> jedisClusterNode = new HashSet<>();\r\nHostAndPort node0 = new HostAndPort(\"127.0.0.1\", 7000);\r\njedisClusterNode.add(node0);\r\nHostAndPort node1 = new HostAndPort(\"127.0.0.1\", 7001);\r\njedisClusterNode.add(node1);\r\nHostAndPort node2 = new HostAndPort(\"127.0.0.1\", 7002);\r\njedisClusterNode.add(node2);\r\nJedisCluster cluster = new JedisCluster(jedisClusterNode);\r\ncluster.set(\"key\", \"value\");\r\nAssert.assertEquals(cluster.get(\"cluster1\"), \"value1\");\r\n```\r\n\r\n3.  Redis cluster reshard slot, the slot 12539 migrate to the node of 7001. But jedis don't recache slots for the node of 7001 because returning in for loop.\r\n\r\n### Redis / Jedis Configuration\r\nIn order to save the machine, All three clusters use '127.0.0.1'. Redis cluster: 127.0.0.1:7000, 127.0.0.1:7001, 127.0.0.1:7002.\r\n\r\nJedisCluster use default configuration.\r\n\r\n#### Jedis version:\r\nv2.9.0\r\n\r\n#### Redis version:\r\nRedis 3.2.4 (00000000/0) 64 bit\r\n\r\n#### Java version:\r\njava version \"1.8.0_92\"\r\nJava(TM) SE Runtime Environment (build 1.8.0_92-b14)\r\nJava HotSpot(TM) 64-Bit Server VM (build 25.92-b14, mixed mode)\r\n\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/xetorthio/jedis/issues/comments/429916484","html_url":"https://github.com/xetorthio/jedis/issues/1877#issuecomment-429916484","issue_url":"https://api.github.com/repos/xetorthio/jedis/issues/1877","id":429916484,"node_id":"MDEyOklzc3VlQ29tbWVudDQyOTkxNjQ4NA==","user":{"login":"sazzad16","id":7600764,"node_id":"MDQ6VXNlcjc2MDA3NjQ=","avatar_url":"https://avatars0.githubusercontent.com/u/7600764?v=4","gravatar_id":"","url":"https://api.github.com/users/sazzad16","html_url":"https://github.com/sazzad16","followers_url":"https://api.github.com/users/sazzad16/followers","following_url":"https://api.github.com/users/sazzad16/following{/other_user}","gists_url":"https://api.github.com/users/sazzad16/gists{/gist_id}","starred_url":"https://api.github.com/users/sazzad16/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sazzad16/subscriptions","organizations_url":"https://api.github.com/users/sazzad16/orgs","repos_url":"https://api.github.com/users/sazzad16/repos","events_url":"https://api.github.com/users/sazzad16/events{/privacy}","received_events_url":"https://api.github.com/users/sazzad16/received_events","type":"User","site_admin":false},"created_at":"2018-10-15T16:11:12Z","updated_at":"2018-10-15T16:11:12Z","author_association":"COLLABORATOR","body":"@lixyou I think you are misjudging. Before your concerned `return;` (after your note), use `System.out.println(slots.size());`. You'll find that it is always `16384`, at least for complete, stable Redis cluster."}}
{"actor":{"display_login":"sazzad16"}}