Skip to content

Commit 24ed19b

Browse files
birdstormZhexuan Yang
authored andcommitted
Fix ScanIterator may fail when an error encounters (#63)
1 parent daca5d2 commit 24ed19b

File tree

2 files changed

+16
-23
lines changed

2 files changed

+16
-23
lines changed

src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,23 +38,22 @@ public RawScanIterator(
3838
}
3939

4040
TiRegion loadCurrentRegionToCache() throws Exception {
41-
TiRegion region;
42-
try (RegionStoreClient client = builder.build(startKey)) {
43-
region = client.getRegion();
44-
BackOffer backOffer = ConcreteBackOffer.newScannerNextMaxBackOff();
45-
if (limit <= 0) {
46-
currentCache = null;
47-
} else {
48-
while (true) {
41+
BackOffer backOffer = ConcreteBackOffer.newScannerNextMaxBackOff();
42+
while (true) {
43+
try (RegionStoreClient client = builder.build(startKey)) {
44+
TiRegion region = client.getRegion();
45+
if (limit <= 0) {
46+
currentCache = null;
47+
} else {
4948
try {
5049
currentCache = client.rawScan(backOffer, startKey, limit);
51-
break;
5250
} catch (final TiKVException e) {
5351
backOffer.doBackOff(BackOffFunction.BackOffFuncType.BoRegionMiss, e);
52+
continue;
5453
}
5554
}
55+
return region;
5656
}
57-
return region;
5857
}
5958
}
6059

src/main/java/org/tikv/common/operation/iterator/ScanIterator.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,8 @@ boolean cacheLoadFails() {
7171
TiRegion region = loadCurrentRegionToCache();
7272
ByteString curRegionEndKey = region.getEndKey();
7373
// currentCache is null means no keys found, whereas currentCache is empty means no values
74-
// found
75-
// the difference lies in whether to continue scanning, because chances are that the same key
76-
// is
77-
// split in another region because of pending entries, region split, e.t.c.
74+
// found. The difference lies in whether to continue scanning, because chances are that
75+
// an empty region exists due to deletion, region split, e.t.c.
7876
// See https://github.com/pingcap/tispark/issues/393 for details
7977
if (currentCache == null) {
8078
return true;
@@ -119,22 +117,18 @@ private Kvrpcpb.KvPair getCurrent() {
119117
if (isCacheDrained()) {
120118
return null;
121119
}
122-
if (index < currentCache.size()) {
123-
--limit;
124-
return currentCache.get(index++);
125-
}
126-
return null;
120+
--limit;
121+
return currentCache.get(index++);
127122
}
128123

129124
@Override
130125
public Kvrpcpb.KvPair next() {
131-
Kvrpcpb.KvPair kv = getCurrent();
132-
if (kv == null) {
133-
// cache drained
126+
Kvrpcpb.KvPair kv;
127+
// continue when cache is empty but not null
128+
for (kv = getCurrent(); currentCache != null && kv == null; kv = getCurrent()) {
134129
if (cacheLoadFails()) {
135130
return null;
136131
}
137-
return getCurrent();
138132
}
139133
return kv;
140134
}

0 commit comments

Comments
 (0)