From 04141962091bd3e3a921f726ea53c283d0fdd38a Mon Sep 17 00:00:00 2001 From: iosmanthus Date: Mon, 28 Feb 2022 15:05:50 +0800 Subject: [PATCH 1/5] rawkv: fix scan return empty set while exist empty key Signed-off-by: iosmanthus --- .../operation/iterator/RawScanIterator.java | 20 ++++++--- .../operation/iterator/ScanIterator.java | 2 +- .../java/org/tikv/raw/RawKVClientTest.java | 41 ++++++++++++++++--- 3 files changed, 51 insertions(+), 12 deletions(-) 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 ce532cb9624..9886004d202 100644 --- a/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java @@ -30,6 +30,7 @@ import org.tikv.kvproto.Kvrpcpb; public class RawScanIterator extends ScanIterator { + private final BackOffer scanBackOffer; public RawScanIterator( @@ -67,11 +68,18 @@ TiRegion loadCurrentRegionToCache() throws GrpcException { } } - private boolean notEndOfScan() { - return limit > 0 - && !(processingLastBatch - && (index >= currentCache.size() - || Key.toRawKey(currentCache.get(index).getKey()).compareTo(endKey) >= 0)); + private boolean eof() { + if (limit <= 0) { + return true; + } + if (!processingLastBatch) { + return false; + } + if (index >= currentCache.size()) { + return true; + } + ByteString lastKey = currentCache.get(index).getKey(); + return !lastKey.isEmpty() && Key.toRawKey(lastKey).compareTo(endKey) >= 0; } boolean isCacheDrained() { @@ -90,7 +98,7 @@ public boolean hasNext() { return false; } } - return notEndOfScan(); + return !eof(); } private Kvrpcpb.KvPair getCurrent() { 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 7d900fe08a6..d99c8b8684d 100644 --- a/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/ScanIterator.java @@ -90,7 +90,7 @@ boolean cacheLoadFails() { Key lastKey = Key.EMPTY; // Session should be single-threaded itself // so that we don't worry about conf change in the middle - // of a transaction. Otherwise below code might lose data + // of a transaction. Otherwise, below code might lose data if (currentCache.size() < limit) { startKey = curRegionEndKey; lastKey = Key.toRawKey(curRegionEndKey); diff --git a/src/test/java/org/tikv/raw/RawKVClientTest.java b/src/test/java/org/tikv/raw/RawKVClientTest.java index bd4b7dd939e..bc23d286054 100644 --- a/src/test/java/org/tikv/raw/RawKVClientTest.java +++ b/src/test/java/org/tikv/raw/RawKVClientTest.java @@ -17,14 +17,31 @@ package org.tikv.raw; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import com.google.protobuf.ByteString; -import java.util.*; -import java.util.concurrent.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import java.util.Random; +import java.util.TreeMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorCompletionService; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import org.apache.commons.lang3.RandomStringUtils; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; @@ -45,6 +62,7 @@ import org.tikv.kvproto.Kvrpcpb; public class RawKVClientTest extends BaseRawKVTest { + private static final String RAW_PREFIX = "raw_\u0001_"; private static final int KEY_POOL_SIZE = 1000000; private static final int TEST_CASES = 10000; @@ -360,13 +378,23 @@ private List rawKeys() { return client.scan(RAW_START_KEY, RAW_END_KEY); } + // https://github.com/tikv/client-java/issues/540 + @Test + public void scanTestForIssue540() { + client.put(ByteString.EMPTY, ByteString.EMPTY); + Assert.assertEquals(1, client.scan(ByteString.EMPTY, 2).size()); + client.delete(ByteString.EMPTY); + } + @Test public void validate() { baseTest(100, 100, 100, 100, false, false, false, false, false); baseTest(100, 100, 100, 100, false, true, true, true, true); } - /** Example of benchmarking base test */ + /** + * Example of benchmarking base test + */ public void benchmark() { baseTest(TEST_CASES, TEST_CASES, 200, 5000, true, false, false, false, false); baseTest(TEST_CASES, TEST_CASES, 200, 5000, true, true, true, true, true); @@ -449,7 +477,9 @@ private void prepare() { int i = cnt; completionService.submit( () -> { - for (int j = 0; j < base; j++) checkDelete(remainingKeys.get(i * base + j).getKey()); + for (int j = 0; j < base; j++) { + checkDelete(remainingKeys.get(i * base + j).getKey()); + } return null; }); } @@ -955,6 +985,7 @@ private static ByteString rawValue(String value) { } private static class ByteStringComparator implements Comparator { + @Override public int compare(ByteString startKey, ByteString endKey) { return FastByteComparisons.compareTo(startKey.toByteArray(), endKey.toByteArray()); From 48575217d5f8af7c9b03b2db6997f2de8eb5d6ce Mon Sep 17 00:00:00 2001 From: iosmanthus Date: Mon, 28 Feb 2022 15:09:28 +0800 Subject: [PATCH 2/5] rawkv: ./dev/javafmt Signed-off-by: iosmanthus --- src/test/java/org/tikv/raw/RawKVClientTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/java/org/tikv/raw/RawKVClientTest.java b/src/test/java/org/tikv/raw/RawKVClientTest.java index bc23d286054..f26ee7ee743 100644 --- a/src/test/java/org/tikv/raw/RawKVClientTest.java +++ b/src/test/java/org/tikv/raw/RawKVClientTest.java @@ -392,9 +392,7 @@ public void validate() { baseTest(100, 100, 100, 100, false, true, true, true, true); } - /** - * Example of benchmarking base test - */ + /** Example of benchmarking base test */ public void benchmark() { baseTest(TEST_CASES, TEST_CASES, 200, 5000, true, false, false, false, false); baseTest(TEST_CASES, TEST_CASES, 200, 5000, true, true, true, true, true); From c4a4a0cd33f8c52053493e4de59afdfea2be3d20 Mon Sep 17 00:00:00 2001 From: iosmanthus Date: Mon, 28 Feb 2022 15:44:40 +0800 Subject: [PATCH 3/5] rawkv: clean tikv data before scanTest Signed-off-by: iosmanthus --- src/test/java/org/tikv/raw/RawKVClientTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/org/tikv/raw/RawKVClientTest.java b/src/test/java/org/tikv/raw/RawKVClientTest.java index f26ee7ee743..3d6e3e70f41 100644 --- a/src/test/java/org/tikv/raw/RawKVClientTest.java +++ b/src/test/java/org/tikv/raw/RawKVClientTest.java @@ -381,7 +381,9 @@ private List rawKeys() { // https://github.com/tikv/client-java/issues/540 @Test public void scanTestForIssue540() { + client.deleteRange(ByteString.EMPTY, ByteString.EMPTY); client.put(ByteString.EMPTY, ByteString.EMPTY); + logger.info(client.scan(ByteString.EMPTY, 2).toString()); Assert.assertEquals(1, client.scan(ByteString.EMPTY, 2).size()); client.delete(ByteString.EMPTY); } From a38972e5529ea1cd9e4b8170cc49a1f79523c803 Mon Sep 17 00:00:00 2001 From: iosmanthus Date: Mon, 28 Feb 2022 15:46:02 +0800 Subject: [PATCH 4/5] rawkv: rename eof to endOfScan Signed-off-by: iosmanthus --- .../org/tikv/common/operation/iterator/RawScanIterator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 9886004d202..ae3951494ad 100644 --- a/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java @@ -68,7 +68,7 @@ TiRegion loadCurrentRegionToCache() throws GrpcException { } } - private boolean eof() { + private boolean endOfScan() { if (limit <= 0) { return true; } @@ -98,7 +98,7 @@ public boolean hasNext() { return false; } } - return !eof(); + return !endOfScan(); } private Kvrpcpb.KvPair getCurrent() { From a62f09eca4fa82d493bf51ead75709cbc3f59e60 Mon Sep 17 00:00:00 2001 From: iosmanthus Date: Mon, 28 Feb 2022 21:38:20 +0800 Subject: [PATCH 5/5] rawkv: remove extra cond Signed-off-by: iosmanthus --- .../operation/iterator/RawScanIterator.java | 6 ----- .../java/org/tikv/raw/RawKVClientTest.java | 27 ++++++++++++++++--- 2 files changed, 23 insertions(+), 10 deletions(-) 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 ae3951494ad..09f17c99fb5 100644 --- a/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java +++ b/src/main/java/org/tikv/common/operation/iterator/RawScanIterator.java @@ -69,15 +69,9 @@ TiRegion loadCurrentRegionToCache() throws GrpcException { } private boolean endOfScan() { - if (limit <= 0) { - return true; - } if (!processingLastBatch) { return false; } - if (index >= currentCache.size()) { - return true; - } ByteString lastKey = currentCache.get(index).getKey(); return !lastKey.isEmpty() && Key.toRawKey(lastKey).compareTo(endKey) >= 0; } diff --git a/src/test/java/org/tikv/raw/RawKVClientTest.java b/src/test/java/org/tikv/raw/RawKVClientTest.java index 3d6e3e70f41..c81c6fc97a1 100644 --- a/src/test/java/org/tikv/raw/RawKVClientTest.java +++ b/src/test/java/org/tikv/raw/RawKVClientTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.google.common.collect.ImmutableList; import com.google.protobuf.ByteString; import java.util.ArrayList; import java.util.Arrays; @@ -378,14 +379,32 @@ private List rawKeys() { return client.scan(RAW_START_KEY, RAW_END_KEY); } - // https://github.com/tikv/client-java/issues/540 @Test public void scanTestForIssue540() { + ByteString splitKeyA = ByteString.copyFromUtf8("splitKeyA"); + ByteString splitKeyB = ByteString.copyFromUtf8("splitKeyB"); + session.splitRegionAndScatter( + ImmutableList.of(splitKeyA.toByteArray(), splitKeyB.toByteArray())); client.deleteRange(ByteString.EMPTY, ByteString.EMPTY); + + client.put(ByteString.EMPTY, ByteString.EMPTY); + client.put(splitKeyA, ByteString.EMPTY); + Assert.assertEquals(0, client.scan(ByteString.EMPTY, 0).size()); + Assert.assertEquals(1, client.scan(ByteString.EMPTY, 1).size()); + Assert.assertEquals(2, client.scan(ByteString.EMPTY, 2).size()); + Assert.assertEquals(2, client.scan(ByteString.EMPTY, 3).size()); + + client.deleteRange(ByteString.EMPTY, ByteString.EMPTY); + client.put(ByteString.EMPTY, ByteString.EMPTY); - logger.info(client.scan(ByteString.EMPTY, 2).toString()); - Assert.assertEquals(1, client.scan(ByteString.EMPTY, 2).size()); - client.delete(ByteString.EMPTY); + client.put(splitKeyA, ByteString.EMPTY); + client.put(splitKeyA.concat(ByteString.copyFromUtf8("1")), ByteString.EMPTY); + client.put(splitKeyA.concat(ByteString.copyFromUtf8("2")), ByteString.EMPTY); + client.put(splitKeyA.concat(ByteString.copyFromUtf8("3")), ByteString.EMPTY); + client.put(splitKeyB.concat(ByteString.copyFromUtf8("1")), ByteString.EMPTY); + Assert.assertEquals(6, client.scan(ByteString.EMPTY, 7).size()); + Assert.assertEquals(0, client.scan(ByteString.EMPTY, -1).size()); + client.deleteRange(ByteString.EMPTY, ByteString.EMPTY); } @Test