This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new dec3026 KUDU-3273: check result of TableLocationsCache.get() for null
dec3026 is described below
commit dec30269cac84e0cbc589145a7685f24e0976592
Author: Li Zhiming <[email protected]>
AuthorDate: Thu Apr 8 16:36:29 2021 +0800
KUDU-3273: check result of TableLocationsCache.get() for null
For TableLocationsCache, if get(key) is called immediately after
cacheTabletLocations(key), it should not return null.
But if partitions change and a call to cacheTabletLocations(otherKey)
occurs between calls to cacheTableLocations(key) and get(key), the
latter might return null.
Change-Id: I13475701804ea4ae2bf8089a1e2b9143a12d2ab9
Reviewed-on: http://gerrit.cloudera.org:8080/17288
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
.../org/apache/kudu/client/AsyncKuduClient.java | 5 ++--
.../org/apache/kudu/client/TestRemoteTablet.java | 13 ++++++++-
.../kudu/client/TestTableLocationsCache.java | 31 ++++++++++++++++++++++
.../java/org/apache/kudu/test/ProtobufUtils.java | 14 ++++++++++
4 files changed, 60 insertions(+), 3 deletions(-)
diff --git
a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
index 34996a5..90edce4 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
@@ -2427,9 +2427,10 @@ public class AsyncKuduClient implements AutoCloseable {
// right away. If not, we throw an exception that RetryRpcErrback will
understand as needing to
// sleep before retrying.
TableLocationsCache.Entry entry = locationsCache.get(requestPartitionKey);
- if (!entry.isNonCoveredRange() && entry.getTablet().getLeaderServerInfo()
== null) {
+ if (entry != null && !entry.isNonCoveredRange() &&
+ entry.getTablet().getLeaderServerInfo() == null) {
throw new NoLeaderFoundException(
- Status.NotFound("Tablet " + entry.toString() + " doesn't have a
leader"));
+ Status.NotFound("Tablet " + entry + " doesn't have a leader"));
}
}
diff --git
a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
index 94aa31b..c3e063a 100644
---
a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
+++
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
@@ -257,7 +257,18 @@ public class TestRemoteTablet {
static RemoteTablet getTablet(int leaderIndex,
int localReplicaIndex,
int sameLocationReplicaIndex) {
- Partition partition =
ProtobufHelper.pbToPartition(ProtobufUtils.getFakePartitionPB().build());
+ return getTablet(
+ leaderIndex, localReplicaIndex, sameLocationReplicaIndex,
+ AsyncKuduClient.EMPTY_ARRAY, AsyncKuduClient.EMPTY_ARRAY);
+ }
+
+ static RemoteTablet getTablet(int leaderIndex,
+ int localReplicaIndex,
+ int sameLocationReplicaIndex,
+ byte[] partitionKeyStart,
+ byte[] partitionKeyEnd) {
+ Partition partition = ProtobufHelper.pbToPartition(
+ ProtobufUtils.getFakePartitionPB(partitionKeyStart,
partitionKeyEnd).build());
List<LocatedTablet.Replica> replicas = new ArrayList<>();
List<ServerInfo> servers = new ArrayList<>();
for (int i = 0; i < 3; i++) {
diff --git
a/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java
index b232bd4..0f0ce1a 100644
---
a/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java
+++
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java
@@ -18,7 +18,10 @@
package org.apache.kudu.client;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import java.nio.charset.StandardCharsets;
import java.util.List;
import com.google.common.base.Ticker;
@@ -67,4 +70,32 @@ public class TestTableLocationsCache {
"ttl=90, tablet=" + tablet.toString() + "}]",
cache.toString());
}
+
+ /**
+ * Test to reproduce the issue in KUDU-3273.
+ *
+ * For TableLocationsCache, if get(key) is called immediately after
+ * cacheTabletLocations(key), it should not return null.
+ * But if partitions change and a call to cacheTabletLocations(otherKey)
+ * occurs between calls to cacheTableLocations(key) and get(key),
+ * the latter might return null.
+ */
+ @Test
+ public void testGet() {
+ byte[] keyB = "b".getBytes(StandardCharsets.UTF_8);
+ List<RemoteTablet> tablets = ImmutableList.of(
+ TestRemoteTablet.getTablet(0, 1, -1, keyB,
AsyncKuduClient.EMPTY_ARRAY));
+ cache.cacheTabletLocations(tablets, AsyncKuduClient.EMPTY_ARRAY, 1, 100);
+ assertNotNull(cache.get(AsyncKuduClient.EMPTY_ARRAY));
+
+ byte[] keyA = "a".getBytes(StandardCharsets.UTF_8);
+ tablets = ImmutableList.of(
+ TestRemoteTablet.getTablet(0, 1, -1, keyA,
AsyncKuduClient.EMPTY_ARRAY));
+ cache.cacheTabletLocations(tablets, keyA, 1, 100);
+
+ assertNull(cache.get(AsyncKuduClient.EMPTY_ARRAY));
+
+ cache.cacheTabletLocations(tablets, AsyncKuduClient.EMPTY_ARRAY, 1, 100);
+ assertNotNull(cache.get(AsyncKuduClient.EMPTY_ARRAY));
+ }
}
diff --git
a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ProtobufUtils.java
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ProtobufUtils.java
index 862439a..93a79a7 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ProtobufUtils.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/ProtobufUtils.java
@@ -41,6 +41,20 @@ public class ProtobufUtils {
}
/**
+ * Get a PartitionPB with specified start and end keys.
+ * @param partitionKeyStart start key
+ * @param partitionKeyEnd end key
+ * @return a fake partition
+ */
+ public static Common.PartitionPB.Builder getFakePartitionPB(
+ byte[] partitionKeyStart, byte[] partitionKeyEnd) {
+ Common.PartitionPB.Builder partition = Common.PartitionPB.newBuilder();
+ partition.setPartitionKeyStart(ByteString.copyFrom(partitionKeyStart));
+ partition.setPartitionKeyEnd(ByteString.copyFrom(partitionKeyEnd));
+ return partition;
+ }
+
+ /**
* Create a InternedReplicaPB based on the passed information.
* @param tsInfoIndex server's index in the TSInfoPB list
* @param role server's role in the configuration
