This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 1830fe2055 HDDS-10367. Fix possible NPE in listKeysLight, listStatus,
listStatusLight (#6221)
1830fe2055 is described below
commit 1830fe20556f608b7945df52d959f36f91c4350b
Author: Ivan Zlenko <[email protected]>
AuthorDate: Wed Feb 28 17:27:39 2024 +0500
HDDS-10367. Fix possible NPE in listKeysLight, listStatus, listStatusLight
(#6221)
---
...OzoneManagerProtocolClientSideTranslatorPB.java | 45 +++++-----
.../hadoop/ozone/om/TestListKeysWithFSO.java | 27 ++++++
.../org/apache/hadoop/ozone/om/TestListStatus.java | 99 ++++++++++------------
3 files changed, 96 insertions(+), 75 deletions(-)
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
index dd201a4262..08fa029833 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
@@ -265,7 +265,6 @@ public final class
OzoneManagerProtocolClientSideTranslatorPB
private OmTransport transport;
private ThreadLocal<S3Auth> threadLocalS3Auth
= new ThreadLocal<>();
-
private boolean s3AuthCheck;
public static final int BLOCK_ALLOCATION_RETRY_COUNT = 5;
@@ -1044,7 +1043,7 @@ public final class
OzoneManagerProtocolClientSideTranslatorPB
reqBuilder.setBucketName(bucketName);
reqBuilder.setCount(maxKeys);
- if (StringUtils.isNotEmpty(startKey)) {
+ if (startKey != null) {
reqBuilder.setStartKey(startKey);
}
@@ -2288,16 +2287,9 @@ public final class
OzoneManagerProtocolClientSideTranslatorPB
.setSortDatanodes(args.getSortDatanodes())
.setLatestVersionLocation(args.getLatestVersionLocation())
.build();
- ListStatusRequest.Builder listStatusRequestBuilder =
- ListStatusRequest.newBuilder()
- .setKeyArgs(keyArgs)
- .setRecursive(recursive)
- .setStartKey(startKey)
- .setNumEntries(numEntries);
- if (allowPartialPrefixes) {
- listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes);
- }
+ ListStatusRequest.Builder listStatusRequestBuilder =
createListStatusRequestBuilder(keyArgs, recursive, startKey,
+ numEntries, allowPartialPrefixes);
OMRequest omRequest = createOMRequest(Type.ListStatus)
.setListStatusRequest(listStatusRequestBuilder.build())
@@ -2324,16 +2316,9 @@ public final class
OzoneManagerProtocolClientSideTranslatorPB
.setSortDatanodes(false)
.setLatestVersionLocation(true)
.build();
- ListStatusRequest.Builder listStatusRequestBuilder =
- ListStatusRequest.newBuilder()
- .setKeyArgs(keyArgs)
- .setRecursive(recursive)
- .setStartKey(startKey)
- .setNumEntries(numEntries);
- if (allowPartialPrefixes) {
- listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes);
- }
+ ListStatusRequest.Builder listStatusRequestBuilder =
createListStatusRequestBuilder(keyArgs, recursive, startKey,
+ numEntries, allowPartialPrefixes);
OMRequest omRequest = createOMRequest(Type.ListStatusLight)
.setListStatusRequest(listStatusRequestBuilder.build())
@@ -2350,6 +2335,26 @@ public final class
OzoneManagerProtocolClientSideTranslatorPB
return statusList;
}
+ private ListStatusRequest.Builder createListStatusRequestBuilder(KeyArgs
keyArgs, boolean recursive, String startKey,
+ long numEntries, boolean allowPartialPrefixes) {
+ ListStatusRequest.Builder listStatusRequestBuilder =
+ ListStatusRequest.newBuilder()
+ .setKeyArgs(keyArgs)
+ .setRecursive(recursive)
+ .setNumEntries(numEntries);
+
+ if (startKey != null) {
+ listStatusRequestBuilder.setStartKey(startKey);
+ } else {
+ listStatusRequestBuilder.setStartKey("");
+ }
+
+ if (allowPartialPrefixes) {
+ listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes);
+ }
+ return listStatusRequestBuilder;
+ }
+
@Override
public List<OzoneFileStatus> listStatus(OmKeyArgs args, boolean recursive,
String startKey, long numEntries) throws IOException {
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
index f499e3569c..11594f3ef1 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
@@ -63,6 +63,8 @@ public class TestListKeysWithFSO {
private static OzoneBucket fsoOzoneBucket;
private static OzoneBucket legacyOzoneBucket2;
private static OzoneBucket fsoOzoneBucket2;
+ private static OzoneBucket emptyLegacyOzoneBucket;
+ private static OzoneBucket emptyFsoOzoneBucket;
private static OzoneClient client;
/**
@@ -105,6 +107,10 @@ public class TestListKeysWithFSO {
ozoneVolume.createBucket(fsoBucketName, omBucketArgs);
fsoOzoneBucket2 = ozoneVolume.getBucket(fsoBucketName);
+ fsoBucketName = "bucket" + RandomStringUtils.randomNumeric(5);
+ ozoneVolume.createBucket(fsoBucketName, omBucketArgs);
+ emptyFsoOzoneBucket = ozoneVolume.getBucket(fsoBucketName);
+
builder = BucketArgs.newBuilder();
builder.setStorageType(StorageType.DISK);
builder.setBucketLayout(BucketLayout.LEGACY);
@@ -113,6 +119,10 @@ public class TestListKeysWithFSO {
ozoneVolume.createBucket(legacyBucketName, omBucketArgs);
legacyOzoneBucket2 = ozoneVolume.getBucket(legacyBucketName);
+ legacyBucketName = "bucket" + RandomStringUtils.randomNumeric(5);
+ ozoneVolume.createBucket(legacyBucketName, omBucketArgs);
+ emptyLegacyOzoneBucket = ozoneVolume.getBucket(legacyBucketName);
+
initFSNameSpace();
}
@@ -479,6 +489,23 @@ public class TestListKeysWithFSO {
expectedKeys =
getExpectedKeyShallowList(keyPrefix, startKey, legacyOzoneBucket);
checkKeyShallowList(keyPrefix, startKey, expectedKeys, fsoOzoneBucket);
+
+ // case-7: keyPrefix corresponds to multiple existing keys and
+ // startKey is null in empty bucket
+ keyPrefix = "a1/b1/c12";
+ startKey = null;
+ // a1/b1/c1222.tx
+ expectedKeys =
+ getExpectedKeyShallowList(keyPrefix, startKey, emptyLegacyOzoneBucket);
+ checkKeyShallowList(keyPrefix, startKey, expectedKeys,
emptyFsoOzoneBucket);
+
+ // case-8: keyPrefix corresponds to multiple existing keys and
+ // startKey is null
+ keyPrefix = "a1/b1/c12";
+ // a1/b1/c1222.tx
+ expectedKeys =
+ getExpectedKeyShallowList(keyPrefix, startKey, legacyOzoneBucket);
+ checkKeyShallowList(keyPrefix, startKey, expectedKeys, fsoOzoneBucket);
}
/**
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java
index 52cb9287cc..20977f9d48 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java
@@ -16,10 +16,10 @@
*/
package org.apache.hadoop.ozone.om;
-import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.hdds.client.RatisReplicationConfig;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.ozone.MiniOzoneCluster;
import org.apache.hadoop.ozone.TestDataUtil;
import org.apache.hadoop.ozone.client.OzoneBucket;
@@ -29,24 +29,30 @@ import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.List;
+import java.util.stream.Stream;
+
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.
- OZONE_FS_ITERATE_BATCH_SIZE;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
/**
* A simple test that asserts that list status output is sorted.
*/
@Timeout(1200)
public class TestListStatus {
+ private static final Logger LOG =
LoggerFactory.getLogger(TestListStatus.class);
private static MiniOzoneCluster cluster = null;
- private static OzoneConfiguration conf;
private static OzoneBucket fsoOzoneBucket;
private static OzoneClient client;
@@ -54,11 +60,11 @@ public class TestListStatus {
* Create a MiniDFSCluster for testing.
* <p>
*
- * @throws IOException
+ * @throws IOException in case of I/O error
*/
@BeforeAll
public static void init() throws Exception {
- conf = new OzoneConfiguration();
+ OzoneConfiguration conf = new OzoneConfiguration();
conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS,
true);
cluster = MiniOzoneCluster.newBuilder(conf).build();
@@ -69,7 +75,7 @@ public class TestListStatus {
fsoOzoneBucket = TestDataUtil
.createVolumeAndBucket(client, BucketLayout.FILE_SYSTEM_OPTIMIZED);
- // Set the number of keys to be processed during batch operate.
+ // Set the number of keys to be processed during batch operated.
conf.setInt(OZONE_FS_ITERATE_BATCH_SIZE, 5);
buildNameSpaceTree(fsoOzoneBucket);
@@ -83,44 +89,30 @@ public class TestListStatus {
}
}
- @Test
- public void testSortedListStatus() throws Exception {
- // a) test if output is sorted
- checkKeyList("", "", 1000, 10, false);
-
- // b) number of keys returns is expected
- checkKeyList("", "", 2, 2, false);
-
- // c) check if full prefix works
- checkKeyList("a1", "", 100, 3, false);
-
- // d) check if full prefix with numEntries work
- checkKeyList("a1", "", 2, 2, false);
-
- // e) check if existing start key >>>
- checkKeyList("a1", "a1/a12", 100, 2, false);
-
- // f) check with non-existing start key
- checkKeyList("", "a7", 100, 6, false);
-
- // g) check if half prefix works
- checkKeyList("b", "", 100, 4, true);
-
- // h) check half prefix with non-existing start key
- checkKeyList("b", "b5", 100, 2, true);
-
- // i) check half prefix with non-existing parent in start key
- checkKeyList("b", "c", 100, 0, true);
-
- // i) check half prefix with non-existing parent in start key
- checkKeyList("b", "b/g5", 100, 4, true);
-
- // i) check half prefix with non-existing parent in start key
- checkKeyList("b", "c/g5", 100, 0, true);
+ @MethodSource("sortedListStatusParametersSource")
+ @ParameterizedTest(name = "{index} {5}")
+ public void testSortedListStatus(String keyPrefix, String startKey, int
numEntries, int expectedNumKeys,
+ boolean isPartialPrefix, String testName)
throws Exception {
+ checkKeyList(keyPrefix, startKey, numEntries, expectedNumKeys,
isPartialPrefix);
+ }
- // j) check prefix with non-existing prefix key
- // and non-existing parent in start key
- checkKeyList("a1/a111", "a1/a111/a100", 100, 0, true);
+ private static Stream<Arguments> sortedListStatusParametersSource() {
+ return Stream.of(
+ arguments("", "", 1000, 10, false, "Test if output is sorted"),
+ arguments("", "", 2, 2, false, "Number of keys returns is expected"),
+ arguments("a1", "", 100, 3, false, "Check if the full prefix works"),
+ arguments("a1", "", 2, 2, false, "Check if full prefix with numEntries
work"),
+ arguments("a1", "a1/a12", 100, 2, false, "Check if existing start key
>>>"),
+ arguments("", "a7", 100, 6, false, "Check with a non-existing start
key"),
+ arguments("b", "", 100, 4, true, "Check if half-prefix works"),
+ arguments("b", "b5", 100, 2, true, "Check half prefix with
non-existing start key"),
+ arguments("b", "c", 100, 0, true, "Check half prefix with non-existing
parent in a start key"),
+ arguments("b", "b/g5", 100, 4, true, "Check half prefix with
non-existing parent in a start key"),
+ arguments("b", "c/g5", 100, 0, true, "Check half prefix with
non-existing parent in a start key"),
+ arguments("a1/a111", "a1/a111/a100", 100, 0, true, "Check prefix with
a non-existing prefix key\n" +
+ " and non-existing parent in a start key"),
+ arguments("a1/a111", null, 100, 0, true, "Check start key is null")
+ );
}
private static void createFile(OzoneBucket bucket, String keyName)
@@ -131,6 +123,7 @@ public class TestListStatus {
oos.flush();
}
}
+
private static void buildNameSpaceTree(OzoneBucket ozoneBucket)
throws Exception {
/*
@@ -172,33 +165,29 @@ public class TestListStatus {
createFile(ozoneBucket, "/b8");
}
- private void checkKeyList(String keyPrefix, String startKey,
- long numEntries, int expectedNumKeys,
- boolean isPartialPrefix)
- throws Exception {
+ private void checkKeyList(String keyPrefix, String startKey, long
numEntries, int expectedNumKeys,
+ boolean isPartialPrefix) throws Exception {
List<OzoneFileStatus> statuses =
fsoOzoneBucket.listStatus(keyPrefix, false, startKey,
numEntries, isPartialPrefix);
assertEquals(expectedNumKeys, statuses.size());
- System.out.println("BEGIN:::keyPrefix---> " + keyPrefix + ":::---> " +
- startKey);
+ LOG.info("BEGIN:::keyPrefix---> {} :::---> {}", keyPrefix, startKey);
for (int i = 0; i < statuses.size() - 1; i++) {
OzoneFileStatus stCurr = statuses.get(i);
OzoneFileStatus stNext = statuses.get(i + 1);
- System.out.println("status:" + stCurr);
+ LOG.info("status: {}", stCurr);
assertThat(stCurr.getPath().compareTo(stNext.getPath())).isLessThan(0);
}
if (!statuses.isEmpty()) {
OzoneFileStatus stNext = statuses.get(statuses.size() - 1);
- System.out.println("status:" + stNext);
+ LOG.info("status: {}", stNext);
}
- System.out.println("END:::keyPrefix---> " + keyPrefix + ":::---> " +
- startKey);
+ LOG.info("END:::keyPrefix---> {}:::---> {}", keyPrefix, startKey);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
