From e3559f69bbaf646396753cb44eb398f016116bfe Mon Sep 17 00:00:00 2001 From: TrafalgarRicardoLu Date: Mon, 27 Jun 2022 11:22:16 +0800 Subject: [PATCH 1/5] fix region may be missed Signed-off-by: TrafalgarRicardoLu --- .../tikv/common/operation/iterator/ConcreteScanIterator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java b/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java index 8d1659b1d6d..eb6a270f3e9 100644 --- a/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java @@ -75,9 +75,9 @@ TiRegion loadCurrentRegionToCache() throws GrpcException { TiRegion region; try (RegionStoreClient client = builder.build(startKey)) { client.setTimeout(conf.getScanTimeout()); - region = client.getRegion(); BackOffer backOffer = ConcreteBackOffer.newScannerNextMaxBackOff(); currentCache = client.scan(backOffer, startKey, version); + region = client.getRegion(); return region; } } From d2409efd480a27a904820b87f1c788f1559a3b99 Mon Sep 17 00:00:00 2001 From: TrafalgarRicardoLu Date: Tue, 28 Jun 2022 16:45:24 +0800 Subject: [PATCH 2/5] fix region may miss in rawscan Signed-off-by: TrafalgarRicardoLu --- .../tikv/common/operation/iterator/ConcreteScanIterator.java | 4 ++++ .../org/tikv/common/operation/iterator/RawScanIterator.java | 3 +++ src/main/java/org/tikv/common/region/RegionStoreClient.java | 3 +++ 3 files changed, 10 insertions(+) diff --git a/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java b/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java index eb6a270f3e9..72422736e76 100644 --- a/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/ConcreteScanIterator.java @@ -77,6 +77,10 @@ TiRegion loadCurrentRegionToCache() throws GrpcException { client.setTimeout(conf.getScanTimeout()); BackOffer backOffer = ConcreteBackOffer.newScannerNextMaxBackOff(); currentCache = client.scan(backOffer, startKey, version); + // If we get region before scan, we will use region from cache which + // may have wrong end key. This may miss some regions that split from old region. + // Client will get the newest region during scan. So we need to + // update region after scan. region = client.getRegion(); return region; } diff --git a/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java b/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java index 5b27132699d..08a64aaf87f 100644 --- a/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java @@ -57,6 +57,9 @@ TiRegion loadCurrentRegionToCache() throws GrpcException { } else { try { currentCache = client.rawScan(backOffer, startKey, limit, keyOnly); + // Client will get the newest region during scan. So we need to + // update region after scan. + region = client.getRegion(); } catch (final TiKVException e) { backOffer.doBackOff(BackOffFunction.BackOffFuncType.BoRegionMiss, e); continue; diff --git a/src/main/java/org/tikv/common/region/RegionStoreClient.java b/src/main/java/org/tikv/common/region/RegionStoreClient.java index 42576fa7ea7..5ba0dd104bc 100644 --- a/src/main/java/org/tikv/common/region/RegionStoreClient.java +++ b/src/main/java/org/tikv/common/region/RegionStoreClient.java @@ -1253,6 +1253,9 @@ public List rawScan(BackOffer backOffer, ByteString key, int limit, bool regionManager, this, resp -> resp.hasRegionError() ? resp.getRegionError() : null); RawScanResponse resp = callWithRetry(backOffer, TikvGrpc.getRawScanMethod(), factory, handler); + // RegionErrorHandler may refresh region cache due to outdated region info, + // This region need to get newest ino from cache. + region = regionManager.getRegionByKey(key, backOffer); return rawScanHelper(resp); } finally { requestTimer.observeDuration(); From 4506ec73c23366e1adcd27303b12dd78431c945b Mon Sep 17 00:00:00 2001 From: TrafalgarRicardoLu Date: Tue, 28 Jun 2022 16:49:03 +0800 Subject: [PATCH 3/5] add todo Signed-off-by: TrafalgarRicardoLu --- .../java/org/tikv/common/operation/iterator/ScanIterator.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java b/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java index dcce68e147d..dc5295462d2 100644 --- a/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java @@ -65,6 +65,8 @@ public abstract class ScanIterator implements Iterator { * * @return TiRegion of current data loaded to cache * @throws GrpcException if scan still fails after backoff + * + * TODO : Add test to check it correctness */ abstract TiRegion loadCurrentRegionToCache() throws GrpcException; From 8583db8df10a6f15a5e9687f9e66af9412bf10e7 Mon Sep 17 00:00:00 2001 From: TrafalgarRicardoLu Date: Wed, 29 Jun 2022 10:09:59 +0800 Subject: [PATCH 4/5] fix typo and format Signed-off-by: TrafalgarRicardoLu --- .../java/org/tikv/common/operation/iterator/ScanIterator.java | 3 +-- src/main/java/org/tikv/common/region/RegionStoreClient.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java b/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java index dc5295462d2..69fd0217fd5 100644 --- a/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java @@ -65,8 +65,7 @@ public abstract class ScanIterator implements Iterator { * * @return TiRegion of current data loaded to cache * @throws GrpcException if scan still fails after backoff - * - * TODO : Add test to check it correctness + *

TODO : Add test to check it correctness */ abstract TiRegion loadCurrentRegionToCache() throws GrpcException; diff --git a/src/main/java/org/tikv/common/region/RegionStoreClient.java b/src/main/java/org/tikv/common/region/RegionStoreClient.java index 5ba0dd104bc..f4fcccbedcf 100644 --- a/src/main/java/org/tikv/common/region/RegionStoreClient.java +++ b/src/main/java/org/tikv/common/region/RegionStoreClient.java @@ -1254,7 +1254,7 @@ public List rawScan(BackOffer backOffer, ByteString key, int limit, bool RawScanResponse resp = callWithRetry(backOffer, TikvGrpc.getRawScanMethod(), factory, handler); // RegionErrorHandler may refresh region cache due to outdated region info, - // This region need to get newest ino from cache. + // This region need to get newest info from cache. region = regionManager.getRegionByKey(key, backOffer); return rawScanHelper(resp); } finally { From 6da3e993403a12efd024c1176493efa5b9b26024 Mon Sep 17 00:00:00 2001 From: TrafalgarRicardoLu Date: Thu, 30 Jun 2022 15:16:08 +0800 Subject: [PATCH 5/5] change the time update region Signed-off-by: TrafalgarRicardoLu --- .../java/org/tikv/common/region/RegionStoreClient.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/tikv/common/region/RegionStoreClient.java b/src/main/java/org/tikv/common/region/RegionStoreClient.java index f4fcccbedcf..ccc820891f1 100644 --- a/src/main/java/org/tikv/common/region/RegionStoreClient.java +++ b/src/main/java/org/tikv/common/region/RegionStoreClient.java @@ -339,9 +339,6 @@ public List scan( BackOffer backOffer, ByteString startKey, long version, boolean keyOnly) { boolean forWrite = false; while (true) { - // we should refresh region - region = regionManager.getRegionByKey(startKey, backOffer); - Supplier request = () -> ScanRequest.newBuilder() @@ -365,6 +362,10 @@ public List scan( version, forWrite); ScanResponse resp = callWithRetry(backOffer, TikvGrpc.getKvScanMethod(), request, handler); + // retry may refresh region info + // we need to update region after retry + region = regionManager.getRegionByKey(startKey, backOffer); + if (isScanSuccess(backOffer, resp)) { return doScan(resp); }