From 87a27d2c7693c80281c3f010e3e8a5b54a0829bd Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Mon, 31 Oct 2022 14:42:32 +0800 Subject: [PATCH 1/8] select tiflash randomly --- .../com/pingcap/tikv/region/RegionManager.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index aaf0c8a277..a057383adf 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -33,10 +33,8 @@ import com.pingcap.tikv.util.BackOffer; import com.pingcap.tikv.util.ConcreteBackOffer; import com.pingcap.tikv.util.Pair; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; + +import java.util.*; import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -110,17 +108,21 @@ public Pair getRegionStorePairByKey( Peer leader = region.getLeader(); store = cache.getStoreById(leader.getStoreId(), backOffer); } else { - outerLoop: + List tiflashStores = new ArrayList<>(); for (Peer peer : region.getLearnerList()) { Store s = getStoreById(peer.getStoreId(), backOffer); for (Metapb.StoreLabel label : s.getLabelsList()) { if (label.getKey().equals(storeType.getLabelKey()) - && label.getValue().equals(storeType.getLabelValue())) { - store = s; - break outerLoop; + && label.getValue().equals(storeType.getLabelValue())) { + tiflashStores.add(s); } } } + // select a tiflash randomly + Random random = new Random(); + int randomIndex = random.nextInt(tiflashStores.size()); + store = tiflashStores.get(randomIndex); + if (store == null) { // clear the region cache so we may get the learner peer next time cache.invalidateRange(region.getStartKey(), region.getEndKey()); From 0f3a8b4da544d815e81869f89b24b4c2f5da8fa3 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Mon, 31 Oct 2022 14:45:47 +0800 Subject: [PATCH 2/8] fmt --- .../src/main/java/com/pingcap/tikv/region/RegionManager.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index a057383adf..12a11a6918 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -33,7 +33,6 @@ import com.pingcap.tikv.util.BackOffer; import com.pingcap.tikv.util.ConcreteBackOffer; import com.pingcap.tikv.util.Pair; - import java.util.*; import java.util.function.Function; import org.slf4j.Logger; @@ -113,7 +112,7 @@ public Pair getRegionStorePairByKey( Store s = getStoreById(peer.getStoreId(), backOffer); for (Metapb.StoreLabel label : s.getLabelsList()) { if (label.getKey().equals(storeType.getLabelKey()) - && label.getValue().equals(storeType.getLabelValue())) { + && label.getValue().equals(storeType.getLabelValue())) { tiflashStores.add(s); } } From 20e2c5e4b49e5194f3a72f19ca334ceeb2392fd4 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Mon, 31 Oct 2022 15:12:06 +0800 Subject: [PATCH 3/8] fix --- .../main/java/com/pingcap/tikv/region/RegionManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index 12a11a6918..482ac992bc 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -118,9 +118,9 @@ public Pair getRegionStorePairByKey( } } // select a tiflash randomly - Random random = new Random(); - int randomIndex = random.nextInt(tiflashStores.size()); - store = tiflashStores.get(randomIndex); + if(tiflashStores.size() > 0) { + store = tiflashStores.get(new Random().nextInt(tiflashStores.size())); + } if (store == null) { // clear the region cache so we may get the learner peer next time From d686582d68dfd7ca7a51b478a76011f1830b92d7 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Mon, 31 Oct 2022 15:42:26 +0800 Subject: [PATCH 4/8] use RR strategy --- .../java/com/pingcap/tikv/region/RegionManager.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index 482ac992bc..c2bee28548 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -51,6 +51,8 @@ public class RegionManager { private final Function cacheInvalidateCallback; + private int TiFlashStoreIndex = 0; + // To avoid double retrieval, we used the async version of grpc // When rpc not returned, instead of call again, it wait for previous one done public RegionManager( @@ -117,9 +119,12 @@ public Pair getRegionStorePairByKey( } } } - // select a tiflash randomly - if(tiflashStores.size() > 0) { - store = tiflashStores.get(new Random().nextInt(tiflashStores.size())); + + // select a tiflash with RR strategy + if (tiflashStores.size() > 0) { + store = + tiflashStores.get(TiFlashStoreIndex > tiflashStores.size() - 1 ? 0 : TiFlashStoreIndex); + TiFlashStoreIndex++; } if (store == null) { @@ -136,6 +141,8 @@ public Pair getRegionStorePairByKey( return Pair.create(region, store); } + public Store balanceTiFlashStore() {} + public Store getStoreById(long id) { return getStoreById(id, ConcreteBackOffer.newGetBackOff()); } From 44826d97eb019c2eb7b65c5df412f305ea47dee4 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Mon, 31 Oct 2022 16:21:42 +0800 Subject: [PATCH 5/8] add test --- .../pingcap/tikv/region/RegionManager.java | 2 - .../test/java/com/pingcap/tikv/GrpcUtils.java | 6 ++ .../com/pingcap/tikv/RegionManagerTest.java | 57 ++++++++++++++++++- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index c2bee28548..86dad8d973 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -141,8 +141,6 @@ public Pair getRegionStorePairByKey( return Pair.create(region, store); } - public Store balanceTiFlashStore() {} - public Store getStoreById(long id) { return getStoreById(id, ConcreteBackOffer.newGetBackOff()); } diff --git a/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java b/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java index e588cebf16..a43e60a365 100644 --- a/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java +++ b/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java @@ -21,6 +21,8 @@ import com.pingcap.tikv.codec.Codec.BytesCodec; import com.pingcap.tikv.codec.CodecDataOutput; import java.util.Arrays; + +import org.tikv.kvproto.Metapb; import org.tikv.kvproto.Metapb.Peer; import org.tikv.kvproto.Metapb.Region; import org.tikv.kvproto.Metapb.RegionEpoch; @@ -65,6 +67,10 @@ public static Peer makePeer(long id, long storeId) { return Peer.newBuilder().setStoreId(storeId).setId(id).build(); } + public static Peer makeLearnerPeer(long id, long storeId) { + return Peer.newBuilder().setRole(Metapb.PeerRole.Learner).setStoreId(storeId).setId(id).build(); + } + public static ByteString encodeKey(byte[] key) { CodecDataOutput cdo = new CodecDataOutput(); BytesCodec.writeBytes(cdo, key); diff --git a/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java b/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java index 8a0b6f546d..6b61c4d337 100644 --- a/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java +++ b/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java @@ -23,6 +23,7 @@ import com.google.protobuf.ByteString; import com.pingcap.tikv.region.RegionManager; import com.pingcap.tikv.region.TiRegion; +import com.pingcap.tikv.region.TiStoreType; import com.pingcap.tikv.util.Pair; import java.io.IOException; import org.junit.Before; @@ -106,7 +107,7 @@ public void getStoreByKey() { GrpcUtils.makeStoreLabel("k2", "v2")))); Pair pair = mgr.getRegionStorePairByKey(searchKey); assertEquals(pair.first.getId(), regionId); - assertEquals(pair.first.getId(), storeId); + assertEquals(pair.second.getId(), storeId); } @Test @@ -143,4 +144,58 @@ public void getStoreById() { } catch (Exception ignored) { } } + + @Test + public void getRegionStorePairByKeyWithTiFlash() { + + ByteString startKey = ByteString.copyFrom(new byte[] {1}); + ByteString endKey = ByteString.copyFrom(new byte[] {10}); + ByteString searchKey = ByteString.copyFrom(new byte[] {5}); + String testAddress = "testAddress"; + long firstStoreId = 233; + long secondStoreId = 234; + int confVer = 1026; + int ver = 1027; + long regionId = 233; + pdServer.addGetRegionResp( + GrpcUtils.makeGetRegionResponse( + pdServer.getClusterId(), + GrpcUtils.makeRegion( + regionId, + GrpcUtils.encodeKey(startKey.toByteArray()), + GrpcUtils.encodeKey(endKey.toByteArray()), + GrpcUtils.makeRegionEpoch(confVer, ver), + GrpcUtils.makeLearnerPeer(1, firstStoreId), + GrpcUtils.makeLearnerPeer(2, secondStoreId)))); + pdServer.addGetStoreResp( + GrpcUtils.makeGetStoreResponse( + pdServer.getClusterId(), + GrpcUtils.makeStore( + firstStoreId, + testAddress, + Metapb.StoreState.Up, + GrpcUtils.makeStoreLabel("engine", "tiflash"), + GrpcUtils.makeStoreLabel("k1", "v1"), + GrpcUtils.makeStoreLabel("k2", "v2")))); + + + pdServer.addGetStoreResp( + GrpcUtils.makeGetStoreResponse( + pdServer.getClusterId(), + GrpcUtils.makeStore( + secondStoreId, + testAddress, + StoreState.Up, + GrpcUtils.makeStoreLabel("engine", "tiflash"), + GrpcUtils.makeStoreLabel("k1", "v1"), + GrpcUtils.makeStoreLabel("k2", "v2")))); + + Pair pair = mgr.getRegionStorePairByKey(searchKey, TiStoreType.TiFlash); + assertEquals(pair.first.getId(), regionId); + assertEquals(pair.second.getId(), firstStoreId); + + Pair secondPair = mgr.getRegionStorePairByKey(searchKey, TiStoreType.TiFlash); + assertEquals(secondPair.first.getId(), regionId); + assertEquals(secondPair.second.getId(), secondStoreId); + } } From c662e6ebdb26a798f89321524fd56edfb0855026 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Mon, 31 Oct 2022 17:28:43 +0800 Subject: [PATCH 6/8] fix --- .../main/java/com/pingcap/tikv/region/RegionManager.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index 86dad8d973..fff9b95dd1 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -34,6 +34,7 @@ import com.pingcap.tikv.util.ConcreteBackOffer; import com.pingcap.tikv.util.Pair; import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,7 +52,7 @@ public class RegionManager { private final Function cacheInvalidateCallback; - private int TiFlashStoreIndex = 0; + private AtomicInteger tiflashStoreIndex = new AtomicInteger(0); // To avoid double retrieval, we used the async version of grpc // When rpc not returned, instead of call again, it wait for previous one done @@ -122,9 +123,7 @@ public Pair getRegionStorePairByKey( // select a tiflash with RR strategy if (tiflashStores.size() > 0) { - store = - tiflashStores.get(TiFlashStoreIndex > tiflashStores.size() - 1 ? 0 : TiFlashStoreIndex); - TiFlashStoreIndex++; + store = tiflashStores.get(tiflashStoreIndex.getAndIncrement() % tiflashStores.size()); } if (store == null) { From 7c111b7a7caee86d82fde32a900da140fd2ccb76 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Tue, 1 Nov 2022 17:59:02 +0800 Subject: [PATCH 7/8] avoid negative number --- .../pingcap/tikv/region/RegionManager.java | 5 +- .../test/java/com/pingcap/tikv/GrpcUtils.java | 1 - .../com/pingcap/tikv/RegionManagerTest.java | 55 +++++++++---------- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index fff9b95dd1..32ffda2080 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -121,9 +121,10 @@ public Pair getRegionStorePairByKey( } } - // select a tiflash with RR strategy + // select a tiflash with Round-Robin strategy if (tiflashStores.size() > 0) { - store = tiflashStores.get(tiflashStoreIndex.getAndIncrement() % tiflashStores.size()); + store = + tiflashStores.get(Math.abs(tiflashStoreIndex.getAndIncrement() % tiflashStores.size())); } if (store == null) { diff --git a/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java b/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java index a43e60a365..20fe0fa40e 100644 --- a/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java +++ b/tikv-client/src/test/java/com/pingcap/tikv/GrpcUtils.java @@ -21,7 +21,6 @@ import com.pingcap.tikv.codec.Codec.BytesCodec; import com.pingcap.tikv.codec.CodecDataOutput; import java.util.Arrays; - import org.tikv.kvproto.Metapb; import org.tikv.kvproto.Metapb.Peer; import org.tikv.kvproto.Metapb.Region; diff --git a/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java b/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java index 6b61c4d337..2bdaa16cc9 100644 --- a/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java +++ b/tikv-client/src/test/java/com/pingcap/tikv/RegionManagerTest.java @@ -158,37 +158,36 @@ public void getRegionStorePairByKeyWithTiFlash() { int ver = 1027; long regionId = 233; pdServer.addGetRegionResp( - GrpcUtils.makeGetRegionResponse( - pdServer.getClusterId(), - GrpcUtils.makeRegion( - regionId, - GrpcUtils.encodeKey(startKey.toByteArray()), - GrpcUtils.encodeKey(endKey.toByteArray()), - GrpcUtils.makeRegionEpoch(confVer, ver), - GrpcUtils.makeLearnerPeer(1, firstStoreId), - GrpcUtils.makeLearnerPeer(2, secondStoreId)))); + GrpcUtils.makeGetRegionResponse( + pdServer.getClusterId(), + GrpcUtils.makeRegion( + regionId, + GrpcUtils.encodeKey(startKey.toByteArray()), + GrpcUtils.encodeKey(endKey.toByteArray()), + GrpcUtils.makeRegionEpoch(confVer, ver), + GrpcUtils.makeLearnerPeer(1, firstStoreId), + GrpcUtils.makeLearnerPeer(2, secondStoreId)))); pdServer.addGetStoreResp( - GrpcUtils.makeGetStoreResponse( - pdServer.getClusterId(), - GrpcUtils.makeStore( - firstStoreId, - testAddress, - Metapb.StoreState.Up, - GrpcUtils.makeStoreLabel("engine", "tiflash"), - GrpcUtils.makeStoreLabel("k1", "v1"), - GrpcUtils.makeStoreLabel("k2", "v2")))); - + GrpcUtils.makeGetStoreResponse( + pdServer.getClusterId(), + GrpcUtils.makeStore( + firstStoreId, + testAddress, + Metapb.StoreState.Up, + GrpcUtils.makeStoreLabel("engine", "tiflash"), + GrpcUtils.makeStoreLabel("k1", "v1"), + GrpcUtils.makeStoreLabel("k2", "v2")))); pdServer.addGetStoreResp( - GrpcUtils.makeGetStoreResponse( - pdServer.getClusterId(), - GrpcUtils.makeStore( - secondStoreId, - testAddress, - StoreState.Up, - GrpcUtils.makeStoreLabel("engine", "tiflash"), - GrpcUtils.makeStoreLabel("k1", "v1"), - GrpcUtils.makeStoreLabel("k2", "v2")))); + GrpcUtils.makeGetStoreResponse( + pdServer.getClusterId(), + GrpcUtils.makeStore( + secondStoreId, + testAddress, + StoreState.Up, + GrpcUtils.makeStoreLabel("engine", "tiflash"), + GrpcUtils.makeStoreLabel("k1", "v1"), + GrpcUtils.makeStoreLabel("k2", "v2")))); Pair pair = mgr.getRegionStorePairByKey(searchKey, TiStoreType.TiFlash); assertEquals(pair.first.getId(), regionId); From 77d3c07282dfdfbeea98a0925a5ce41a3d76ef61 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Tue, 1 Nov 2022 19:32:38 +0800 Subject: [PATCH 8/8] avoid negative number --- .../src/main/java/com/pingcap/tikv/region/RegionManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java index 32ffda2080..5888aed055 100644 --- a/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java +++ b/tikv-client/src/main/java/com/pingcap/tikv/region/RegionManager.java @@ -124,7 +124,8 @@ public Pair getRegionStorePairByKey( // select a tiflash with Round-Robin strategy if (tiflashStores.size() > 0) { store = - tiflashStores.get(Math.abs(tiflashStoreIndex.getAndIncrement() % tiflashStores.size())); + tiflashStores.get( + Math.floorMod(tiflashStoreIndex.getAndIncrement(), tiflashStores.size())); } if (store == null) {