From bc373f93aa2dd3b6961e9f363fbf833f76f1ff74 Mon Sep 17 00:00:00 2001 From: baiyina Date: Thu, 12 Sep 2024 11:57:30 +0800 Subject: [PATCH 1/2] Modify the consistent hash 1. Modified the consistent hashing algorithm to fix a bug where only one real node was being accessed. 2. Updated the corresponding unit tests to reflect the changes." --- .../AbstractConsistentHash.java | 6 ++ .../SortArrayMapConsistentHash.java | 7 +- .../consistenthash/TreeMapConsistentHash.java | 8 +- .../consistenthash/RangeCheckTestUtil.java | 15 +++ .../SortArrayMapConsistentHashTest.java | 100 +++++++++++++----- .../TreeMapConsistentHashTest.java | 69 ++++++++++-- 6 files changed, 162 insertions(+), 43 deletions(-) create mode 100644 cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/RangeCheckTestUtil.java diff --git a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java index 73307b36..8731cf6f 100644 --- a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java +++ b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java @@ -21,6 +21,11 @@ public abstract class AbstractConsistentHash { */ protected abstract void add(long key,String value); + /** + * Clear old data in the structure + */ + protected abstract void clear(); + /** * 排序节点，数据结构自身支持排序可以不用重写 */ @@ -41,6 +46,7 @@ protected void sort(){} */ public String process(List values,String key){ + clear(); for (String value : values) { add(hash(value), value); } diff --git a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java index ed7f17ee..97a8f902 100644 --- a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java +++ b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java @@ -20,8 +20,6 @@ public class SortArrayMapConsistentHash extends AbstractConsistentHash { @Override public void add(long key, String value) { - // fix https://github.com/crossoverJie/cim/issues/79 - sortArrayMap.clear(); for (int i = 0; i < VIRTUAL_NODE_SIZE; i++) { Long hash = super.hash("vir" + key + i); sortArrayMap.add(hash,value); @@ -34,6 +32,11 @@ public void sort() { sortArrayMap.sort(); } + @Override + protected void clear() { + sortArrayMap.clear(); + } + @Override public String getFirstNodeValue(String value) { long hash = super.hash(value); diff --git a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java index 73f9869d..ae587899 100644 --- a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java +++ b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java @@ -23,9 +23,6 @@ public class TreeMapConsistentHash extends AbstractConsistentHash { @Override public void add(long key, String value) { - - // fix https://github.com/crossoverJie/cim/issues/79 - treeMap.clear(); for (int i = 0; i < VIRTUAL_NODE_SIZE; i++) { Long hash = super.hash("vir" + key + i); treeMap.put(hash,value); @@ -33,6 +30,11 @@ public void add(long key, String value) { treeMap.put(key, value); } + @Override + protected void clear() { + treeMap.clear(); + } + @Override public String getFirstNodeValue(String value) { long hash = super.hash(value); diff --git a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/RangeCheckTestUtil.java b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/RangeCheckTestUtil.java new file mode 100644 index 00000000..b36ebee0 --- /dev/null +++ b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/RangeCheckTestUtil.java @@ -0,0 +1,15 @@ +package com.crossoverjie.cim.common.route.algorithm.consistenthash; + +import org.junit.Assert; + +/** + * @description: TODO + * @author: zhangguoa + * @date: 2024/9/12 9:58 + * @project: cim + */ +public class RangeCheckTestUtil { + public static void assertInRange (int value, int l, int r) { + Assert.assertTrue(value >= l && value <= r); + } +} diff --git a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java index 863e38c2..8b11cc57 100644 --- a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java +++ b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java @@ -1,10 +1,11 @@ package com.crossoverjie.cim.common.route.algorithm.consistenthash; +import com.crossoverjie.cim.common.data.construct.SortArrayMap; import org.junit.Assert; import org.junit.Test; -import java.util.ArrayList; -import java.util.List; +import java.lang.reflect.Field; +import java.util.*; public class SortArrayMapConsistentHashTest { @@ -16,10 +17,11 @@ public void getFirstNodeValue() { for (int i = 0; i < 10; i++) { strings.add("127.0.0." + i) ; } - String process = map.process(strings,"zhangsan"); - System.out.println(process); - Assert.assertEquals("127.0.0.9",process); - + String PROCESS = map.process(strings, "zhangsan"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "zhangsan"); + Assert.assertEquals(PROCESS, process); + } } @Test @@ -30,10 +32,12 @@ public void getFirstNodeValue2() { for (int i = 0; i < 10; i++) { strings.add("127.0.0." + i) ; } - String process = map.process(strings,"zhangsan2"); - System.out.println(process); - Assert.assertEquals("127.0.0.9",process); + String PROCESS = map.process(strings,"zhangsan2"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "zhangsan2"); + Assert.assertEquals(PROCESS, process); + } } @Test @@ -44,10 +48,11 @@ public void getFirstNodeValue3() { for (int i = 0; i < 10; i++) { strings.add("127.0.0." + i) ; } - String process = map.process(strings,"1551253899106"); - - System.out.println(process); - Assert.assertEquals("127.0.0.9",process); + String PROCESS = map.process(strings,"1551253899106"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "1551253899106"); + Assert.assertEquals(PROCESS, process); + } } @@ -59,10 +64,12 @@ public void getFirstNodeValue4() { strings.add("45.78.28.220:9000:8081") ; strings.add("45.78.28.220:9100:9081") ; - String process = map.process(strings,"1551253899106"); - System.out.println(process); - Assert.assertEquals("45.78.28.220:9100:9081",process); + String PROCESS = map.process(strings,"1551253899106"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "1551253899106"); + Assert.assertEquals(PROCESS, process); + } } @Test public void getFirstNodeValue5() { @@ -73,10 +80,11 @@ public void getFirstNodeValue5() { strings.add("45.78.28.220:9100:9081") ; strings.add("45.78.28.220:9100:10081") ; - String process = map.process(strings,"1551253899106"); - - System.out.println(process); - Assert.assertEquals("45.78.28.220:9100:10081",process); + String PROCESS = map.process(strings,"1551253899106"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "1551253899106"); + Assert.assertEquals(PROCESS, process); + } } @Test @@ -88,10 +96,11 @@ public void getFirstNodeValue6() { strings.add("45.78.28.220:9100:9081") ; strings.add("45.78.28.220:9100:10081") ; - String process = map.process(strings,"1551253899106"); - - System.out.println(process); - Assert.assertEquals("45.78.28.220:9100:10081",process); + String PROCESS = map.process(strings,"1551253899106"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "1551253899106"); + Assert.assertEquals(PROCESS, process); + } } @Test public void getFirstNodeValue7() { @@ -103,12 +112,49 @@ public void getFirstNodeValue7() { strings.add("45.78.28.220:9100:10081") ; strings.add("45.78.28.220:9100:00081") ; - String process = map.process(strings,"1551253899106"); + String PROCESS = map.process(strings,"1551253899106"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "1551253899106"); + Assert.assertEquals(PROCESS, process); + } + } + + @Test + public void getFirstNodeValue8() { + AbstractConsistentHash map = new SortArrayMapConsistentHash() ; - System.out.println(process); - Assert.assertEquals("45.78.28.220:9100:00081",process); + List strings = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + strings.add("127.0.0." + i); + } + Set processes = new HashSet<>(); + for (int i = 0; i < 10; i++) { + String process = map.process(strings,"zhangsan" + i); + processes.add(process); + } + RangeCheckTestUtil.assertInRange(processes.size(), 2, 10); } + @Test + public void testVirtualNode() throws NoSuchFieldException, IllegalAccessException { + AbstractConsistentHash map = new SortArrayMapConsistentHash(); + List strings = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + strings.add("127.0.0." + i); + } + + String process = map.process(strings,"zhangsan"); + Field sortArrayMapField = SortArrayMapConsistentHash.class.getDeclaredField("sortArrayMap"); + sortArrayMapField.setAccessible(true); + Field virtualNodeSizeField = SortArrayMapConsistentHash.class.getDeclaredField("VIRTUAL_NODE_SIZE"); + virtualNodeSizeField.setAccessible(true); + + SortArrayMap sortArrayMap = (SortArrayMap) sortArrayMapField.get(map); + int virtualNodeSize = (int) virtualNodeSizeField.get(map); + + System.out.println("sortArrayMapSize = " + sortArrayMap.size() + "\n" + "virtualNodeSize = " + virtualNodeSize); + Assert.assertEquals(sortArrayMap.size(), (virtualNodeSize + 1) * 10); + } } \ No newline at end of file diff --git a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java index 4bc9bbe5..c5a36830 100644 --- a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java +++ b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java @@ -3,11 +3,13 @@ import org.junit.Assert; import org.junit.Test; -import java.util.ArrayList; -import java.util.List; +import java.lang.reflect.Field; +import java.util.*; public class TreeMapConsistentHashTest { + + @Test public void getFirstNodeValue() { AbstractConsistentHash map = new TreeMapConsistentHash() ; @@ -16,9 +18,11 @@ public void getFirstNodeValue() { for (int i = 0; i < 10; i++) { strings.add("127.0.0." + i) ; } - String process = map.process(strings,"zhangsan"); - System.out.println(process); - Assert.assertEquals("127.0.0.9",process); + String PROCESS = map.process(strings, "zhangsan"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "zhangsan"); + Assert.assertEquals(PROCESS, process); + } } @@ -31,10 +35,14 @@ public void getFirstNodeValue2() { for (int i = 0; i < 10; i++) { strings.add("127.0.0." + i) ; } - String process = map.process(strings,"zhangsan2"); - System.out.println(process); + String PROCESS = map.process(strings,"zhangsan2"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "zhangsan2"); + Assert.assertEquals(PROCESS, process); + } + - Assert.assertEquals("127.0.0.9",process); +// Assert.assertEquals("127.0.0.9",process); } @@ -46,9 +54,48 @@ public void getFirstNodeValue3() { for (int i = 0; i < 10; i++) { strings.add("127.0.0." + i) ; } - String process = map.process(strings,"1551253899106"); + String PROCESS = map.process(strings,"1551253899106"); + for (int i = 0; i < 100; i++) { + String process = map.process(strings, "1551253899106"); + Assert.assertEquals(PROCESS, process); + } + } + + @Test + public void getFirstNodeValue4() { + AbstractConsistentHash map = new TreeMapConsistentHash() ; + + List strings = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + strings.add("127.0.0." + i); + } + Set processes = new HashSet<>(); + for (int i = 0; i < 10; i++) { + String process = map.process(strings,"zhangsan" + i); + processes.add(process); + } + RangeCheckTestUtil.assertInRange(processes.size(), 2, 10); + } + + @Test + public void testVirtualNode() throws NoSuchFieldException, IllegalAccessException { + AbstractConsistentHash map = new TreeMapConsistentHash(); + + List strings = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + strings.add("127.0.0." + i); + } + + String process = map.process(strings,"zhangsan"); + Field treeMapField = TreeMapConsistentHash.class.getDeclaredField("treeMap"); + treeMapField.setAccessible(true); + Field virtualNodeSizeField = TreeMapConsistentHash.class.getDeclaredField("VIRTUAL_NODE_SIZE"); + virtualNodeSizeField.setAccessible(true); + + TreeMap treeMap = (TreeMap) treeMapField.get(map); + int virtualNodeSize = (int) virtualNodeSizeField.get(map); - System.out.println(process); - Assert.assertEquals("127.0.0.9",process); + System.out.println("treeMapSize = " + treeMap.size() + "\n" + "virtualNodeSize = " + virtualNodeSize); + Assert.assertEquals(treeMap.size(), (virtualNodeSize + 1) * 10); } } \ No newline at end of file From 758cfebc58182ddbb15bdec4bea7fc60836b173e Mon Sep 17 00:00:00 2001 From: baiyina Date: Thu, 12 Sep 2024 21:34:30 +0800 Subject: [PATCH 2/2] Modify the consistent hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on crossoverJie’s review, made code modifications --- .../consistenthash/AbstractConsistentHash.java | 2 +- .../consistenthash/SortArrayMapConsistentHash.java | 12 ++++++++++++ .../consistenthash/TreeMapConsistentHash.java | 11 +++++++++++ .../SortArrayMapConsistentHashTest.java | 10 +++------- .../consistenthash/TreeMapConsistentHashTest.java | 13 +++---------- 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java index 8731cf6f..34e92b6d 100644 --- a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java +++ b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/AbstractConsistentHash.java @@ -45,7 +45,7 @@ protected void sort(){} * @return */ public String process(List values,String key){ - + // fix https://github.com/crossoverJie/cim/issues/79 clear(); for (String value : values) { add(hash(value), value); diff --git a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java index 97a8f902..73c6f011 100644 --- a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java +++ b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHash.java @@ -1,6 +1,9 @@ package com.crossoverjie.cim.common.route.algorithm.consistenthash; import com.crossoverjie.cim.common.data.construct.SortArrayMap; +import com.google.common.annotations.VisibleForTesting; + +import java.util.TreeMap; /** * Function:自定义排序 Map 实现 @@ -32,6 +35,15 @@ public void sort() { sortArrayMap.sort(); } + /** + * Used only in test. + * @return Return the data structure of the current algorithm. + */ + @VisibleForTesting + public SortArrayMap getSortArrayMap() { + return sortArrayMap; + } + @Override protected void clear() { sortArrayMap.clear(); diff --git a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java index ae587899..650ba766 100644 --- a/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java +++ b/cim-common/src/main/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHash.java @@ -2,6 +2,7 @@ import com.crossoverjie.cim.common.enums.StatusEnum; import com.crossoverjie.cim.common.exception.CIMException; +import com.google.common.annotations.VisibleForTesting; import java.util.SortedMap; import java.util.TreeMap; @@ -35,6 +36,15 @@ protected void clear() { treeMap.clear(); } + /** + * Used only in test. + * @return Return the data structure of the current algorithm. + */ + @VisibleForTesting + public TreeMap getTreeMap() { + return treeMap; + } + @Override public String getFirstNodeValue(String value) { long hash = super.hash(value); @@ -48,4 +58,5 @@ public String getFirstNodeValue(String value) { } return treeMap.firstEntry().getValue(); } + } diff --git a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java index 8b11cc57..97df099e 100644 --- a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java +++ b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/SortArrayMapConsistentHashTest.java @@ -137,7 +137,7 @@ public void getFirstNodeValue8() { @Test public void testVirtualNode() throws NoSuchFieldException, IllegalAccessException { - AbstractConsistentHash map = new SortArrayMapConsistentHash(); + SortArrayMapConsistentHash map = new SortArrayMapConsistentHash(); List strings = new ArrayList<>(); for (int i = 0; i < 10; i++) { @@ -145,13 +145,9 @@ public void testVirtualNode() throws NoSuchFieldException, IllegalAccessExceptio } String process = map.process(strings,"zhangsan"); - Field sortArrayMapField = SortArrayMapConsistentHash.class.getDeclaredField("sortArrayMap"); - sortArrayMapField.setAccessible(true); - Field virtualNodeSizeField = SortArrayMapConsistentHash.class.getDeclaredField("VIRTUAL_NODE_SIZE"); - virtualNodeSizeField.setAccessible(true); - SortArrayMap sortArrayMap = (SortArrayMap) sortArrayMapField.get(map); - int virtualNodeSize = (int) virtualNodeSizeField.get(map); + SortArrayMap sortArrayMap = map.getSortArrayMap(); + int virtualNodeSize = 2; System.out.println("sortArrayMapSize = " + sortArrayMap.size() + "\n" + "virtualNodeSize = " + virtualNodeSize); Assert.assertEquals(sortArrayMap.size(), (virtualNodeSize + 1) * 10); diff --git a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java index c5a36830..01060894 100644 --- a/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java +++ b/cim-common/src/test/java/com/crossoverjie/cim/common/route/algorithm/consistenthash/TreeMapConsistentHashTest.java @@ -40,9 +40,6 @@ public void getFirstNodeValue2() { String process = map.process(strings, "zhangsan2"); Assert.assertEquals(PROCESS, process); } - - -// Assert.assertEquals("127.0.0.9",process); } @@ -79,7 +76,7 @@ public void getFirstNodeValue4() { @Test public void testVirtualNode() throws NoSuchFieldException, IllegalAccessException { - AbstractConsistentHash map = new TreeMapConsistentHash(); + TreeMapConsistentHash map = new TreeMapConsistentHash(); List strings = new ArrayList<>(); for (int i = 0; i < 10; i++) { @@ -87,13 +84,9 @@ public void testVirtualNode() throws NoSuchFieldException, IllegalAccessExceptio } String process = map.process(strings,"zhangsan"); - Field treeMapField = TreeMapConsistentHash.class.getDeclaredField("treeMap"); - treeMapField.setAccessible(true); - Field virtualNodeSizeField = TreeMapConsistentHash.class.getDeclaredField("VIRTUAL_NODE_SIZE"); - virtualNodeSizeField.setAccessible(true); - TreeMap treeMap = (TreeMap) treeMapField.get(map); - int virtualNodeSize = (int) virtualNodeSizeField.get(map); + TreeMap treeMap = map.getTreeMap(); + int virtualNodeSize = 2; System.out.println("treeMapSize = " + treeMap.size() + "\n" + "virtualNodeSize = " + virtualNodeSize); Assert.assertEquals(treeMap.size(), (virtualNodeSize + 1) * 10);