From 046d6a87997345faeca8bf4e1dcd1d74cee33a91 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Tue, 15 Apr 2025 18:38:43 +0300 Subject: [PATCH 01/14] HDDS-10284. Added @TempDir to TestMiniOzoneCluster#testMultipleDataDirs --- .../java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index 6e26a3ae7ac6..8839efeee149 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -26,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; +import java.nio.file.Path; import java.io.IOException; import java.util.ArrayList; import java.util.HashSet; @@ -45,7 +46,6 @@ import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; import org.apache.hadoop.ozone.container.common.statemachine.EndpointStateMachine; import org.apache.hadoop.ozone.container.common.volume.StorageVolume; -import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -60,6 +60,7 @@ public class TestMiniOzoneCluster { private MiniOzoneCluster cluster; private static OzoneConfiguration conf; + private static Path tempDir; @BeforeAll static void setup(@TempDir File testDir) { @@ -267,7 +268,7 @@ public void testMultipleDataDirs() throws Exception { + "-" + cluster.getClusterId(); assertEquals(name, cluster.getName()); - final String baseDir = GenericTestUtils.getTempPath(name); + final String baseDir = tempDir.resolve(name).toString(); assertEquals(baseDir, cluster.getBaseDir()); From 05cf7c0c00fb50373f700020228ca51d185e20f1 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Tue, 15 Apr 2025 19:02:32 +0300 Subject: [PATCH 02/14] Fixed checkstyle --- .../apache/hadoop/ozone/TestMiniOzoneCluster.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index 8839efeee149..c92a5b6e4aa5 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -26,8 +26,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; -import java.nio.file.Path; import java.io.IOException; +import java.nio.file.Path; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -204,9 +204,11 @@ public void testKeepPortsWhenRestartDN() throws Exception { /** * Test that a DN can register with SCM even if it was started before the SCM. + * * @throws Exception */ - @Test @Timeout(100) + @Test + @Timeout(100) public void testDNstartAfterSCM() throws Exception { // Start a cluster with 3 DN cluster = MiniOzoneCluster.newBuilder(conf) @@ -249,9 +251,11 @@ public void testDNstartAfterSCM() throws Exception { /** * Test that multiple datanode directories are created in MiniOzoneCluster. + * * @throws Exception */ - @Test @Timeout(60) + @Test + @Timeout(60) public void testMultipleDataDirs() throws Exception { // Start a cluster with 3 DN and configure reserved space in each DN String reservedSpace = "1B"; @@ -279,8 +283,8 @@ public void testMultipleDataDirs() throws Exception { assertEquals(3, volumeList.size()); volumeList.forEach(storageVolume -> assertEquals( - (long) StorageSize.parse(reservedSpace).getValue(), - storageVolume.getVolumeUsage().get().getReservedInBytes())); + (long) StorageSize.parse(reservedSpace).getValue(), + storageVolume.getVolumeUsage().get().getReservedInBytes())); } } From 944571bde8d22e8a78c83395dfaef682c477dcd6 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Wed, 16 Apr 2025 10:32:19 +0300 Subject: [PATCH 03/14] HDDS-10284. Fixed checkstyle & @TempDir --- .../java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index c92a5b6e4aa5..d44ebb53188c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -60,7 +60,8 @@ public class TestMiniOzoneCluster { private MiniOzoneCluster cluster; private static OzoneConfiguration conf; - private static Path tempDir; + @TempDir + private Path tempDir; @BeforeAll static void setup(@TempDir File testDir) { From e8e4a1ec1c244bb7d3e98a80915440d74de49509 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Wed, 16 Apr 2025 10:58:43 +0300 Subject: [PATCH 04/14] HDDS-10284. Fixed @TempDir --- .../java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index c92a5b6e4aa5..3087e3e41e6c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -60,7 +60,8 @@ public class TestMiniOzoneCluster { private MiniOzoneCluster cluster; private static OzoneConfiguration conf; - private static Path tempDir; + @TempDir + private Path pathTempDir; @BeforeAll static void setup(@TempDir File testDir) { @@ -272,7 +273,7 @@ public void testMultipleDataDirs() throws Exception { + "-" + cluster.getClusterId(); assertEquals(name, cluster.getName()); - final String baseDir = tempDir.resolve(name).toString(); + final String baseDir = pathTempDir.resolve(name).toString(); assertEquals(baseDir, cluster.getBaseDir()); From a3bdd8cc3c56d961929031706757c40c996e9fcf Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Wed, 16 Apr 2025 17:16:24 +0300 Subject: [PATCH 05/14] Delete GenericTestUtils#getTempPath --- .../apache/ozone/test/GenericTestUtils.java | 28 +--------------- .../hadoop/ozone/TestMiniOzoneCluster.java | 4 +-- .../apache/hadoop/ozone/MiniOzoneCluster.java | 32 +++++++++++++++++-- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java index dedaaa9b3745..c5cf6e1500b5 100644 --- a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java @@ -56,9 +56,8 @@ * Provides some very generic helpers which might be used across the tests. */ public abstract class GenericTestUtils { - public static final String SYSPROP_TEST_DATA_DIR = "test.build.data"; public static final String DEFAULT_TEST_DATA_DIR; - public static final String DEFAULT_TEST_DATA_PATH = "target/test/data/"; + /** * Error string used in * {@link GenericTestUtils#waitFor(BooleanSupplier, int, int)}. @@ -90,31 +89,6 @@ public static Instant getTestStartTime() { return Instant.ofEpochMilli(System.currentTimeMillis()); } - /** - * Get a temp path. This may or may not be relative; it depends on what the - * {@link #SYSPROP_TEST_DATA_DIR} is set to. If unset, it returns a path - * under the relative path {@link #DEFAULT_TEST_DATA_PATH} - * - * @param subpath sub path, with no leading "/" character - * @return a string to use in paths - * - * @deprecated use {@link org.junit.jupiter.api.io.TempDir} instead. - */ - @Deprecated - public static String getTempPath(String subpath) { - String prop = WINDOWS ? DEFAULT_TEST_DATA_PATH - : System.getProperty(SYSPROP_TEST_DATA_DIR, DEFAULT_TEST_DATA_PATH); - - if (prop.isEmpty()) { - // corner case: property is there but empty - prop = DEFAULT_TEST_DATA_PATH; - } - if (!prop.endsWith("/")) { - prop = prop + "/"; - } - return prop + subpath; - } - /** * Wait for the specified test to return true. The test will be performed * initially and then every {@code checkEveryMillis} until at least diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index 3087e3e41e6c..c891fade1ecd 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -60,8 +60,6 @@ public class TestMiniOzoneCluster { private MiniOzoneCluster cluster; private static OzoneConfiguration conf; - @TempDir - private Path pathTempDir; @BeforeAll static void setup(@TempDir File testDir) { @@ -273,7 +271,7 @@ public void testMultipleDataDirs() throws Exception { + "-" + cluster.getClusterId(); assertEquals(name, cluster.getName()); - final String baseDir = pathTempDir.resolve(name).toString(); + final String baseDir = MiniOzoneCluster.getTempPath(name); assertEquals(baseDir, cluster.getBaseDir()); diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index 9fa8b799593c..7c2b245da7d9 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -43,6 +43,10 @@ * Interface used for MiniOzoneClusters. */ public interface MiniOzoneCluster extends AutoCloseable { + String SYSPROP_TEST_DATA_DIR = "test.build.data"; + String DEFAULT_TEST_DATA_PATH = "target/test/data/"; + + boolean WINDOWS = System.getProperty("os.name").startsWith("Windows"); /** * Returns the Builder to construct MiniOzoneCluster. @@ -250,7 +254,30 @@ default String getName() { } default String getBaseDir() { - return GenericTestUtils.getTempPath(getName()); + return getTempPath(getName()); + } + + /** + * Get a temp path. This may or may not be relative; it depends on what the + * {@link #SYSPROP_TEST_DATA_DIR} is set to. If unset, it returns a path + * under the relative path {@link #DEFAULT_TEST_DATA_PATH} + * + * @param subpath sub path, with no leading "/" character + * @return a string to use in paths + * + */ + static String getTempPath(String subpath){ + String prop = WINDOWS ? DEFAULT_TEST_DATA_PATH + : System.getProperty(SYSPROP_TEST_DATA_DIR, DEFAULT_TEST_DATA_PATH); + + if (prop.isEmpty()) { + // corner case: property is there but empty + prop = DEFAULT_TEST_DATA_PATH; + } + if (!prop.endsWith("/")) { + prop = prop + "/"; + } + return prop + subpath; } /** @@ -303,8 +330,7 @@ public Builder setSCMConfigurator(SCMConfigurator configurator) { private void setClusterId() { clusterId = UUID.randomUUID().toString(); - path = GenericTestUtils.getTempPath( - MiniOzoneClusterImpl.class.getSimpleName() + "-" + clusterId); + path = getTempPath(MiniOzoneClusterImpl.class.getSimpleName() + "-" + clusterId); } /** From a6309517afddb6d11b827eab15f1364f47626449 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Wed, 16 Apr 2025 17:19:55 +0300 Subject: [PATCH 06/14] Fix checkstyle --- .../apache/ozone/test/GenericTestUtils.java | 19 +++++++---- .../apache/hadoop/ozone/MiniOzoneCluster.java | 32 ++++++++++--------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java index c5cf6e1500b5..96a71b3ae029 100644 --- a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java @@ -107,7 +107,7 @@ public static Instant getTestStartTime() { * @throws InterruptedException if the method is interrupted while waiting */ public static void waitFor(BooleanSupplier check, int checkEveryMillis, - int waitForMillis) throws TimeoutException, InterruptedException { + int waitForMillis) throws TimeoutException, InterruptedException { Preconditions.checkNotNull(check, ERROR_MISSING_ARGUMENT); Preconditions.checkArgument(waitForMillis >= checkEveryMillis, ERROR_INVALID_ARGUMENT); @@ -150,7 +150,7 @@ public static void setLogLevel(Logger logger, Level level) { } public static void setLogLevel(org.slf4j.Logger logger, - org.slf4j.event.Level level) { + org.slf4j.event.Level level) { setLogLevel(toLog4j(logger), Level.toLevel(level.toString())); } @@ -171,7 +171,7 @@ public static void withLogDisabled(Class clazz, Runnable task) { } public static T mockFieldReflection(Object object, String fieldName) - throws NoSuchFieldException, IllegalAccessException { + throws NoSuchFieldException, IllegalAccessException { Field field = object.getClass().getDeclaredField(fieldName); boolean isAccessible = field.isAccessible(); @@ -191,7 +191,7 @@ public static T mockFieldReflection(Object object, String fieldName) } public static T getFieldReflection(Object object, String fieldName) - throws NoSuchFieldException, IllegalAccessException { + throws NoSuchFieldException, IllegalAccessException { Field field = object.getClass().getDeclaredField(fieldName); boolean isAccessible = field.isAccessible(); @@ -211,7 +211,7 @@ public static T getFieldReflection(Object object, String fieldName) public static Map getReverseMap(Map> map) { return map.entrySet().stream().flatMap(entry -> entry.getValue().stream() .map(v -> Pair.of(v, entry.getKey()))) - .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); + .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); } /** @@ -255,6 +255,7 @@ public void clearOutput() { writer().getBuffer().setLength(0); } } + @Deprecated public static Logger toLog4j(org.slf4j.Logger logger) { return LogManager.getLogger(logger.getName()); @@ -272,7 +273,9 @@ public static PrintStreamCapturer captureErr() { return new SystemErrCapturer(); } - /** Capture contents of a {@code PrintStream}, until {@code close()}d. */ + /** + * Capture contents of a {@code PrintStream}, until {@code close()}d. + */ public abstract static class PrintStreamCapturer implements AutoCloseable, Supplier { private final ByteArrayOutputStream bytes; private final PrintStream bytesPrintStream; @@ -351,6 +354,7 @@ public SystemOutCapturer() { /** * Replaces {@link System#in} with a stream that provides {@code lines} as input. + * * @return an {@code AutoCloseable} to restore the original {@link System#in} stream */ public static AutoCloseable supplyOnSystemIn(String... lines) { @@ -430,9 +434,10 @@ public static final class ReflectionUtils { /** * This method provides the modifiers field using reflection approach which is compatible * for both pre Java 9 and post java 9 versions. + * * @return modifiers field * @throws IllegalAccessException illegalAccessException, - * @throws NoSuchFieldException noSuchFieldException. + * @throws NoSuchFieldException noSuchFieldException. */ public static Field getModifiersField() throws IllegalAccessException, NoSuchFieldException { Field modifiersField = null; diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index 7c2b245da7d9..f49d481d0e21 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -52,7 +52,6 @@ public interface MiniOzoneCluster extends AutoCloseable { * Returns the Builder to construct MiniOzoneCluster. * * @param conf OzoneConfiguration - * * @return MiniOzoneCluster builder */ static Builder newBuilder(OzoneConfiguration conf) { @@ -63,7 +62,6 @@ static Builder newBuilder(OzoneConfiguration conf) { * Returns the Builder to construct MiniOzoneHACluster. * * @param conf OzoneConfiguration - * * @return MiniOzoneCluster builder */ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { @@ -82,7 +80,7 @@ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { * configured {@link HddsDatanodeService} registers with * {@link StorageContainerManager}. * - * @throws TimeoutException In case of timeout + * @throws TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitForClusterToBeReady() throws TimeoutException, InterruptedException; @@ -91,14 +89,14 @@ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { * Waits for at least one RATIS pipeline of given factor to be reported in open * state. * - * @param factor replication factor + * @param factor replication factor * @param timeoutInMs timeout value in milliseconds - * @throws TimeoutException In case of timeout + * @throws TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, int timeoutInMs) - throws TimeoutException, InterruptedException; + throws TimeoutException, InterruptedException; /** * Sets the timeout value after which @@ -111,7 +109,7 @@ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, /** * Waits/blocks till the cluster is out of safe mode. * - * @throws TimeoutException TimeoutException In case of timeout + * @throws TimeoutException TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitTobeOutOfSafeMode() throws TimeoutException, InterruptedException; @@ -162,7 +160,7 @@ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, * {@link StorageContainerManager} associated with the MiniOzoneCluster. */ StorageContainerLocationProtocolClientSideTranslatorPB - getStorageContainerLocationClient() throws IOException; + getStorageContainerLocationClient() throws IOException; /** * Restarts StorageContainerManager instance. @@ -198,6 +196,7 @@ void restartHddsDatanode(int i, boolean waitForDatanode) */ void restartHddsDatanode(DatanodeDetails dn, boolean waitForDatanode) throws InterruptedException, TimeoutException, IOException; + /** * Shutdown a particular HddsDatanode. * @@ -264,9 +263,8 @@ default String getBaseDir() { * * @param subpath sub path, with no leading "/" character * @return a string to use in paths - * */ - static String getTempPath(String subpath){ + static String getTempPath(String subpath) { String prop = WINDOWS ? DEFAULT_TEST_DATA_PATH : System.getProperty(SYSPROP_TEST_DATA_DIR, DEFAULT_TEST_DATA_PATH); @@ -302,7 +300,7 @@ abstract class Builder { protected boolean includeRecon = false; protected int numOfDatanodes = 3; - protected boolean startDataNodes = true; + protected boolean startDataNodes = true; protected CertificateClient certClient; protected SecretKeyClient secretKeyClient; protected DatanodeFactory dnFactory = UniformDatanodesFactory.newBuilder().build(); @@ -316,8 +314,10 @@ protected Builder(OzoneConfiguration conf) { ExitUtils.disableSystemExit(); } - /** Prepare the builder for another call to {@link #build()}, avoiding conflict - * between the clusters created. */ + /** + * Prepare the builder for another call to {@link #build()}, avoiding conflict + * between the clusters created. + */ protected void prepareForNextBuild() { conf = new OzoneConfiguration(conf); setClusterId(); @@ -364,7 +364,6 @@ public Builder setSecretKeyClient(SecretKeyClient client) { * MiniOzoneCluster. * * @param val number of datanodes - * * @return MiniOzoneCluster.Builder */ public Builder setNumDatanodes(int val) { @@ -406,9 +405,12 @@ interface DatanodeFactory extends CheckedFunction Date: Wed, 16 Apr 2025 17:40:39 +0300 Subject: [PATCH 07/14] Fix checkstyle --- .../src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index f49d481d0e21..368d5062fa32 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -35,7 +35,6 @@ import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.recon.ReconServer; import org.apache.hadoop.security.authentication.client.AuthenticationException; -import org.apache.ozone.test.GenericTestUtils; import org.apache.ratis.util.ExitUtils; import org.apache.ratis.util.function.CheckedFunction; From 859b67dd114d0f4e026ee697eb000f42391528a1 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Thu, 17 Apr 2025 11:43:40 +0300 Subject: [PATCH 08/14] Fix checkstyle --- .../test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index c891fade1ecd..483276c5e60c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -27,7 +27,6 @@ import java.io.File; import java.io.IOException; -import java.nio.file.Path; import java.util.ArrayList; import java.util.HashSet; import java.util.List; From 269928e5a084201cb97a2d71c5f79de5d7f04029 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Thu, 17 Apr 2025 12:32:24 +0300 Subject: [PATCH 09/14] Fix checkstyle --- .../src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index 368d5062fa32..f52a8bdf9e9c 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -159,7 +159,7 @@ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, * {@link StorageContainerManager} associated with the MiniOzoneCluster. */ StorageContainerLocationProtocolClientSideTranslatorPB - getStorageContainerLocationClient() throws IOException; + getStorageContainerLocationClient() throws IOException; /** * Restarts StorageContainerManager instance. From cf949bfb7745befd4da852f82d4dd5b596f9c99b Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Thu, 17 Apr 2025 14:32:35 +0300 Subject: [PATCH 10/14] Delete GenericTestUtils#getTempPath --- .../apache/ozone/test/GenericTestUtils.java | 47 ++++----------- .../hadoop/ozone/TestMiniOzoneCluster.java | 15 +++-- .../apache/hadoop/ozone/MiniOzoneCluster.java | 59 ++++++++++++++----- 3 files changed, 65 insertions(+), 56 deletions(-) diff --git a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java index dedaaa9b3745..96a71b3ae029 100644 --- a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java @@ -56,9 +56,8 @@ * Provides some very generic helpers which might be used across the tests. */ public abstract class GenericTestUtils { - public static final String SYSPROP_TEST_DATA_DIR = "test.build.data"; public static final String DEFAULT_TEST_DATA_DIR; - public static final String DEFAULT_TEST_DATA_PATH = "target/test/data/"; + /** * Error string used in * {@link GenericTestUtils#waitFor(BooleanSupplier, int, int)}. @@ -90,31 +89,6 @@ public static Instant getTestStartTime() { return Instant.ofEpochMilli(System.currentTimeMillis()); } - /** - * Get a temp path. This may or may not be relative; it depends on what the - * {@link #SYSPROP_TEST_DATA_DIR} is set to. If unset, it returns a path - * under the relative path {@link #DEFAULT_TEST_DATA_PATH} - * - * @param subpath sub path, with no leading "/" character - * @return a string to use in paths - * - * @deprecated use {@link org.junit.jupiter.api.io.TempDir} instead. - */ - @Deprecated - public static String getTempPath(String subpath) { - String prop = WINDOWS ? DEFAULT_TEST_DATA_PATH - : System.getProperty(SYSPROP_TEST_DATA_DIR, DEFAULT_TEST_DATA_PATH); - - if (prop.isEmpty()) { - // corner case: property is there but empty - prop = DEFAULT_TEST_DATA_PATH; - } - if (!prop.endsWith("/")) { - prop = prop + "/"; - } - return prop + subpath; - } - /** * Wait for the specified test to return true. The test will be performed * initially and then every {@code checkEveryMillis} until at least @@ -133,7 +107,7 @@ public static String getTempPath(String subpath) { * @throws InterruptedException if the method is interrupted while waiting */ public static void waitFor(BooleanSupplier check, int checkEveryMillis, - int waitForMillis) throws TimeoutException, InterruptedException { + int waitForMillis) throws TimeoutException, InterruptedException { Preconditions.checkNotNull(check, ERROR_MISSING_ARGUMENT); Preconditions.checkArgument(waitForMillis >= checkEveryMillis, ERROR_INVALID_ARGUMENT); @@ -176,7 +150,7 @@ public static void setLogLevel(Logger logger, Level level) { } public static void setLogLevel(org.slf4j.Logger logger, - org.slf4j.event.Level level) { + org.slf4j.event.Level level) { setLogLevel(toLog4j(logger), Level.toLevel(level.toString())); } @@ -197,7 +171,7 @@ public static void withLogDisabled(Class clazz, Runnable task) { } public static T mockFieldReflection(Object object, String fieldName) - throws NoSuchFieldException, IllegalAccessException { + throws NoSuchFieldException, IllegalAccessException { Field field = object.getClass().getDeclaredField(fieldName); boolean isAccessible = field.isAccessible(); @@ -217,7 +191,7 @@ public static T mockFieldReflection(Object object, String fieldName) } public static T getFieldReflection(Object object, String fieldName) - throws NoSuchFieldException, IllegalAccessException { + throws NoSuchFieldException, IllegalAccessException { Field field = object.getClass().getDeclaredField(fieldName); boolean isAccessible = field.isAccessible(); @@ -237,7 +211,7 @@ public static T getFieldReflection(Object object, String fieldName) public static Map getReverseMap(Map> map) { return map.entrySet().stream().flatMap(entry -> entry.getValue().stream() .map(v -> Pair.of(v, entry.getKey()))) - .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); + .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); } /** @@ -281,6 +255,7 @@ public void clearOutput() { writer().getBuffer().setLength(0); } } + @Deprecated public static Logger toLog4j(org.slf4j.Logger logger) { return LogManager.getLogger(logger.getName()); @@ -298,7 +273,9 @@ public static PrintStreamCapturer captureErr() { return new SystemErrCapturer(); } - /** Capture contents of a {@code PrintStream}, until {@code close()}d. */ + /** + * Capture contents of a {@code PrintStream}, until {@code close()}d. + */ public abstract static class PrintStreamCapturer implements AutoCloseable, Supplier { private final ByteArrayOutputStream bytes; private final PrintStream bytesPrintStream; @@ -377,6 +354,7 @@ public SystemOutCapturer() { /** * Replaces {@link System#in} with a stream that provides {@code lines} as input. + * * @return an {@code AutoCloseable} to restore the original {@link System#in} stream */ public static AutoCloseable supplyOnSystemIn(String... lines) { @@ -456,9 +434,10 @@ public static final class ReflectionUtils { /** * This method provides the modifiers field using reflection approach which is compatible * for both pre Java 9 and post java 9 versions. + * * @return modifiers field * @throws IllegalAccessException illegalAccessException, - * @throws NoSuchFieldException noSuchFieldException. + * @throws NoSuchFieldException noSuchFieldException. */ public static Field getModifiersField() throws IllegalAccessException, NoSuchFieldException { Field modifiersField = null; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index 6e26a3ae7ac6..483276c5e60c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -45,7 +45,6 @@ import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; import org.apache.hadoop.ozone.container.common.statemachine.EndpointStateMachine; import org.apache.hadoop.ozone.container.common.volume.StorageVolume; -import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -203,9 +202,11 @@ public void testKeepPortsWhenRestartDN() throws Exception { /** * Test that a DN can register with SCM even if it was started before the SCM. + * * @throws Exception */ - @Test @Timeout(100) + @Test + @Timeout(100) public void testDNstartAfterSCM() throws Exception { // Start a cluster with 3 DN cluster = MiniOzoneCluster.newBuilder(conf) @@ -248,9 +249,11 @@ public void testDNstartAfterSCM() throws Exception { /** * Test that multiple datanode directories are created in MiniOzoneCluster. + * * @throws Exception */ - @Test @Timeout(60) + @Test + @Timeout(60) public void testMultipleDataDirs() throws Exception { // Start a cluster with 3 DN and configure reserved space in each DN String reservedSpace = "1B"; @@ -267,7 +270,7 @@ public void testMultipleDataDirs() throws Exception { + "-" + cluster.getClusterId(); assertEquals(name, cluster.getName()); - final String baseDir = GenericTestUtils.getTempPath(name); + final String baseDir = MiniOzoneCluster.getTempPath(name); assertEquals(baseDir, cluster.getBaseDir()); @@ -278,8 +281,8 @@ public void testMultipleDataDirs() throws Exception { assertEquals(3, volumeList.size()); volumeList.forEach(storageVolume -> assertEquals( - (long) StorageSize.parse(reservedSpace).getValue(), - storageVolume.getVolumeUsage().get().getReservedInBytes())); + (long) StorageSize.parse(reservedSpace).getValue(), + storageVolume.getVolumeUsage().get().getReservedInBytes())); } } diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index 9fa8b799593c..f52a8bdf9e9c 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -35,7 +35,6 @@ import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.recon.ReconServer; import org.apache.hadoop.security.authentication.client.AuthenticationException; -import org.apache.ozone.test.GenericTestUtils; import org.apache.ratis.util.ExitUtils; import org.apache.ratis.util.function.CheckedFunction; @@ -43,12 +42,15 @@ * Interface used for MiniOzoneClusters. */ public interface MiniOzoneCluster extends AutoCloseable { + String SYSPROP_TEST_DATA_DIR = "test.build.data"; + String DEFAULT_TEST_DATA_PATH = "target/test/data/"; + + boolean WINDOWS = System.getProperty("os.name").startsWith("Windows"); /** * Returns the Builder to construct MiniOzoneCluster. * * @param conf OzoneConfiguration - * * @return MiniOzoneCluster builder */ static Builder newBuilder(OzoneConfiguration conf) { @@ -59,7 +61,6 @@ static Builder newBuilder(OzoneConfiguration conf) { * Returns the Builder to construct MiniOzoneHACluster. * * @param conf OzoneConfiguration - * * @return MiniOzoneCluster builder */ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { @@ -78,7 +79,7 @@ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { * configured {@link HddsDatanodeService} registers with * {@link StorageContainerManager}. * - * @throws TimeoutException In case of timeout + * @throws TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitForClusterToBeReady() throws TimeoutException, InterruptedException; @@ -87,14 +88,14 @@ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { * Waits for at least one RATIS pipeline of given factor to be reported in open * state. * - * @param factor replication factor + * @param factor replication factor * @param timeoutInMs timeout value in milliseconds - * @throws TimeoutException In case of timeout + * @throws TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, int timeoutInMs) - throws TimeoutException, InterruptedException; + throws TimeoutException, InterruptedException; /** * Sets the timeout value after which @@ -107,7 +108,7 @@ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, /** * Waits/blocks till the cluster is out of safe mode. * - * @throws TimeoutException TimeoutException In case of timeout + * @throws TimeoutException TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitTobeOutOfSafeMode() throws TimeoutException, InterruptedException; @@ -194,6 +195,7 @@ void restartHddsDatanode(int i, boolean waitForDatanode) */ void restartHddsDatanode(DatanodeDetails dn, boolean waitForDatanode) throws InterruptedException, TimeoutException, IOException; + /** * Shutdown a particular HddsDatanode. * @@ -250,7 +252,29 @@ default String getName() { } default String getBaseDir() { - return GenericTestUtils.getTempPath(getName()); + return getTempPath(getName()); + } + + /** + * Get a temp path. This may or may not be relative; it depends on what the + * {@link #SYSPROP_TEST_DATA_DIR} is set to. If unset, it returns a path + * under the relative path {@link #DEFAULT_TEST_DATA_PATH} + * + * @param subpath sub path, with no leading "/" character + * @return a string to use in paths + */ + static String getTempPath(String subpath) { + String prop = WINDOWS ? DEFAULT_TEST_DATA_PATH + : System.getProperty(SYSPROP_TEST_DATA_DIR, DEFAULT_TEST_DATA_PATH); + + if (prop.isEmpty()) { + // corner case: property is there but empty + prop = DEFAULT_TEST_DATA_PATH; + } + if (!prop.endsWith("/")) { + prop = prop + "/"; + } + return prop + subpath; } /** @@ -275,7 +299,7 @@ abstract class Builder { protected boolean includeRecon = false; protected int numOfDatanodes = 3; - protected boolean startDataNodes = true; + protected boolean startDataNodes = true; protected CertificateClient certClient; protected SecretKeyClient secretKeyClient; protected DatanodeFactory dnFactory = UniformDatanodesFactory.newBuilder().build(); @@ -289,8 +313,10 @@ protected Builder(OzoneConfiguration conf) { ExitUtils.disableSystemExit(); } - /** Prepare the builder for another call to {@link #build()}, avoiding conflict - * between the clusters created. */ + /** + * Prepare the builder for another call to {@link #build()}, avoiding conflict + * between the clusters created. + */ protected void prepareForNextBuild() { conf = new OzoneConfiguration(conf); setClusterId(); @@ -303,8 +329,7 @@ public Builder setSCMConfigurator(SCMConfigurator configurator) { private void setClusterId() { clusterId = UUID.randomUUID().toString(); - path = GenericTestUtils.getTempPath( - MiniOzoneClusterImpl.class.getSimpleName() + "-" + clusterId); + path = getTempPath(MiniOzoneClusterImpl.class.getSimpleName() + "-" + clusterId); } /** @@ -338,7 +363,6 @@ public Builder setSecretKeyClient(SecretKeyClient client) { * MiniOzoneCluster. * * @param val number of datanodes - * * @return MiniOzoneCluster.Builder */ public Builder setNumDatanodes(int val) { @@ -380,9 +404,12 @@ interface DatanodeFactory extends CheckedFunction Date: Fri, 18 Apr 2025 10:45:43 +0300 Subject: [PATCH 11/14] Fix formatting --- .../apache/ozone/test/GenericTestUtils.java | 19 ++++------- .../hadoop/ozone/TestMiniOzoneCluster.java | 15 ++++----- .../apache/hadoop/ozone/MiniOzoneCluster.java | 32 +++++++++---------- 3 files changed, 28 insertions(+), 38 deletions(-) diff --git a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java index 96a71b3ae029..c5cf6e1500b5 100644 --- a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java @@ -107,7 +107,7 @@ public static Instant getTestStartTime() { * @throws InterruptedException if the method is interrupted while waiting */ public static void waitFor(BooleanSupplier check, int checkEveryMillis, - int waitForMillis) throws TimeoutException, InterruptedException { + int waitForMillis) throws TimeoutException, InterruptedException { Preconditions.checkNotNull(check, ERROR_MISSING_ARGUMENT); Preconditions.checkArgument(waitForMillis >= checkEveryMillis, ERROR_INVALID_ARGUMENT); @@ -150,7 +150,7 @@ public static void setLogLevel(Logger logger, Level level) { } public static void setLogLevel(org.slf4j.Logger logger, - org.slf4j.event.Level level) { + org.slf4j.event.Level level) { setLogLevel(toLog4j(logger), Level.toLevel(level.toString())); } @@ -171,7 +171,7 @@ public static void withLogDisabled(Class clazz, Runnable task) { } public static T mockFieldReflection(Object object, String fieldName) - throws NoSuchFieldException, IllegalAccessException { + throws NoSuchFieldException, IllegalAccessException { Field field = object.getClass().getDeclaredField(fieldName); boolean isAccessible = field.isAccessible(); @@ -191,7 +191,7 @@ public static T mockFieldReflection(Object object, String fieldName) } public static T getFieldReflection(Object object, String fieldName) - throws NoSuchFieldException, IllegalAccessException { + throws NoSuchFieldException, IllegalAccessException { Field field = object.getClass().getDeclaredField(fieldName); boolean isAccessible = field.isAccessible(); @@ -211,7 +211,7 @@ public static T getFieldReflection(Object object, String fieldName) public static Map getReverseMap(Map> map) { return map.entrySet().stream().flatMap(entry -> entry.getValue().stream() .map(v -> Pair.of(v, entry.getKey()))) - .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); + .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); } /** @@ -255,7 +255,6 @@ public void clearOutput() { writer().getBuffer().setLength(0); } } - @Deprecated public static Logger toLog4j(org.slf4j.Logger logger) { return LogManager.getLogger(logger.getName()); @@ -273,9 +272,7 @@ public static PrintStreamCapturer captureErr() { return new SystemErrCapturer(); } - /** - * Capture contents of a {@code PrintStream}, until {@code close()}d. - */ + /** Capture contents of a {@code PrintStream}, until {@code close()}d. */ public abstract static class PrintStreamCapturer implements AutoCloseable, Supplier { private final ByteArrayOutputStream bytes; private final PrintStream bytesPrintStream; @@ -354,7 +351,6 @@ public SystemOutCapturer() { /** * Replaces {@link System#in} with a stream that provides {@code lines} as input. - * * @return an {@code AutoCloseable} to restore the original {@link System#in} stream */ public static AutoCloseable supplyOnSystemIn(String... lines) { @@ -434,10 +430,9 @@ public static final class ReflectionUtils { /** * This method provides the modifiers field using reflection approach which is compatible * for both pre Java 9 and post java 9 versions. - * * @return modifiers field * @throws IllegalAccessException illegalAccessException, - * @throws NoSuchFieldException noSuchFieldException. + * @throws NoSuchFieldException noSuchFieldException. */ public static Field getModifiersField() throws IllegalAccessException, NoSuchFieldException { Field modifiersField = null; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index 483276c5e60c..4a88cfde9ab5 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -19,6 +19,7 @@ import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port; import static org.apache.hadoop.hdds.protocol.MockDatanodeDetails.randomDatanodeDetails; +import static org.apache.hadoop.ozone.MiniOzoneCluster.getTempPath; import static org.apache.hadoop.ozone.OzoneConfigKeys.HDDS_CONTAINER_RATIS_IPC_RANDOM_PORT; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -202,11 +203,9 @@ public void testKeepPortsWhenRestartDN() throws Exception { /** * Test that a DN can register with SCM even if it was started before the SCM. - * * @throws Exception */ - @Test - @Timeout(100) + @Test @Timeout(100) public void testDNstartAfterSCM() throws Exception { // Start a cluster with 3 DN cluster = MiniOzoneCluster.newBuilder(conf) @@ -249,11 +248,9 @@ public void testDNstartAfterSCM() throws Exception { /** * Test that multiple datanode directories are created in MiniOzoneCluster. - * * @throws Exception */ - @Test - @Timeout(60) + @Test @Timeout(60) public void testMultipleDataDirs() throws Exception { // Start a cluster with 3 DN and configure reserved space in each DN String reservedSpace = "1B"; @@ -270,7 +267,7 @@ public void testMultipleDataDirs() throws Exception { + "-" + cluster.getClusterId(); assertEquals(name, cluster.getName()); - final String baseDir = MiniOzoneCluster.getTempPath(name); + final String baseDir = getTempPath(name); assertEquals(baseDir, cluster.getBaseDir()); @@ -281,8 +278,8 @@ public void testMultipleDataDirs() throws Exception { assertEquals(3, volumeList.size()); volumeList.forEach(storageVolume -> assertEquals( - (long) StorageSize.parse(reservedSpace).getValue(), - storageVolume.getVolumeUsage().get().getReservedInBytes())); + (long) StorageSize.parse(reservedSpace).getValue(), + storageVolume.getVolumeUsage().get().getReservedInBytes())); } } diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index f52a8bdf9e9c..ce187a388c5d 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -35,6 +35,7 @@ import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.recon.ReconServer; import org.apache.hadoop.security.authentication.client.AuthenticationException; +import org.apache.ozone.test.GenericTestUtils; import org.apache.ratis.util.ExitUtils; import org.apache.ratis.util.function.CheckedFunction; @@ -44,13 +45,13 @@ public interface MiniOzoneCluster extends AutoCloseable { String SYSPROP_TEST_DATA_DIR = "test.build.data"; String DEFAULT_TEST_DATA_PATH = "target/test/data/"; - boolean WINDOWS = System.getProperty("os.name").startsWith("Windows"); /** * Returns the Builder to construct MiniOzoneCluster. * * @param conf OzoneConfiguration + * * @return MiniOzoneCluster builder */ static Builder newBuilder(OzoneConfiguration conf) { @@ -61,6 +62,7 @@ static Builder newBuilder(OzoneConfiguration conf) { * Returns the Builder to construct MiniOzoneHACluster. * * @param conf OzoneConfiguration + * * @return MiniOzoneCluster builder */ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { @@ -79,7 +81,7 @@ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { * configured {@link HddsDatanodeService} registers with * {@link StorageContainerManager}. * - * @throws TimeoutException In case of timeout + * @throws TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitForClusterToBeReady() throws TimeoutException, InterruptedException; @@ -88,14 +90,14 @@ static MiniOzoneHAClusterImpl.Builder newHABuilder(OzoneConfiguration conf) { * Waits for at least one RATIS pipeline of given factor to be reported in open * state. * - * @param factor replication factor + * @param factor replication factor * @param timeoutInMs timeout value in milliseconds - * @throws TimeoutException In case of timeout + * @throws TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, int timeoutInMs) - throws TimeoutException, InterruptedException; + throws TimeoutException, InterruptedException; /** * Sets the timeout value after which @@ -108,7 +110,7 @@ void waitForPipelineTobeReady(HddsProtos.ReplicationFactor factor, /** * Waits/blocks till the cluster is out of safe mode. * - * @throws TimeoutException TimeoutException In case of timeout + * @throws TimeoutException TimeoutException In case of timeout * @throws InterruptedException In case of interrupt while waiting */ void waitTobeOutOfSafeMode() throws TimeoutException, InterruptedException; @@ -195,7 +197,6 @@ void restartHddsDatanode(int i, boolean waitForDatanode) */ void restartHddsDatanode(DatanodeDetails dn, boolean waitForDatanode) throws InterruptedException, TimeoutException, IOException; - /** * Shutdown a particular HddsDatanode. * @@ -299,7 +300,7 @@ abstract class Builder { protected boolean includeRecon = false; protected int numOfDatanodes = 3; - protected boolean startDataNodes = true; + protected boolean startDataNodes = true; protected CertificateClient certClient; protected SecretKeyClient secretKeyClient; protected DatanodeFactory dnFactory = UniformDatanodesFactory.newBuilder().build(); @@ -313,10 +314,8 @@ protected Builder(OzoneConfiguration conf) { ExitUtils.disableSystemExit(); } - /** - * Prepare the builder for another call to {@link #build()}, avoiding conflict - * between the clusters created. - */ + /** Prepare the builder for another call to {@link #build()}, avoiding conflict + * between the clusters created. */ protected void prepareForNextBuild() { conf = new OzoneConfiguration(conf); setClusterId(); @@ -329,7 +328,8 @@ public Builder setSCMConfigurator(SCMConfigurator configurator) { private void setClusterId() { clusterId = UUID.randomUUID().toString(); - path = getTempPath(MiniOzoneClusterImpl.class.getSimpleName() + "-" + clusterId); + path = getTempPath( + MiniOzoneClusterImpl.class.getSimpleName() + "-" + clusterId); } /** @@ -363,6 +363,7 @@ public Builder setSecretKeyClient(SecretKeyClient client) { * MiniOzoneCluster. * * @param val number of datanodes + * * @return MiniOzoneCluster.Builder */ public Builder setNumDatanodes(int val) { @@ -404,12 +405,9 @@ interface DatanodeFactory extends CheckedFunction Date: Fri, 18 Apr 2025 12:00:02 +0300 Subject: [PATCH 12/14] Fix checkstyle --- .../src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index ce187a388c5d..42fbe0b2e858 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -35,7 +35,6 @@ import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.recon.ReconServer; import org.apache.hadoop.security.authentication.client.AuthenticationException; -import org.apache.ozone.test.GenericTestUtils; import org.apache.ratis.util.ExitUtils; import org.apache.ratis.util.function.CheckedFunction; From b06a0efcb74abea5ce070b4e0fc9012abdf984da Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Fri, 18 Apr 2025 12:27:15 +0300 Subject: [PATCH 13/14] Clean code --- .../java/org/apache/ozone/test/GenericTestUtils.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java index c5cf6e1500b5..e22f1e47eea2 100644 --- a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java @@ -56,7 +56,6 @@ * Provides some very generic helpers which might be used across the tests. */ public abstract class GenericTestUtils { - public static final String DEFAULT_TEST_DATA_DIR; /** * Error string used in @@ -67,16 +66,8 @@ public abstract class GenericTestUtils { public static final String ERROR_INVALID_ARGUMENT = "Total wait time should be greater than check interval time"; - public static final boolean WINDOWS = - System.getProperty("os.name").startsWith("Windows"); - private static final long NANOSECONDS_PER_MILLISECOND = 1_000_000; - static { - DEFAULT_TEST_DATA_DIR = - "target" + File.separator + "test" + File.separator + "data"; - } - /** * Return current time in millis as an {@code Instant}. This may be * before {@link Instant#now()}, since the latter includes nanoseconds, too. From 7e358d0829db6244b26d9dfb2c43fa46306e7075 Mon Sep 17 00:00:00 2001 From: Anastasia Kostryukova Date: Fri, 18 Apr 2025 13:14:46 +0300 Subject: [PATCH 14/14] Move #getTempPath & const to Builder --- .../apache/ozone/test/GenericTestUtils.java | 1 - .../hadoop/ozone/TestMiniOzoneCluster.java | 3 +- .../apache/hadoop/ozone/MiniOzoneCluster.java | 53 ++++++++++--------- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java index e22f1e47eea2..78d00712a544 100644 --- a/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/test/java/org/apache/ozone/test/GenericTestUtils.java @@ -21,7 +21,6 @@ import com.google.common.base.Preconditions; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.InputStream; import java.io.OutputStream; import java.io.PrintStream; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java index 4a88cfde9ab5..de53b851eb4e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java @@ -19,7 +19,6 @@ import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port; import static org.apache.hadoop.hdds.protocol.MockDatanodeDetails.randomDatanodeDetails; -import static org.apache.hadoop.ozone.MiniOzoneCluster.getTempPath; import static org.apache.hadoop.ozone.OzoneConfigKeys.HDDS_CONTAINER_RATIS_IPC_RANDOM_PORT; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -267,7 +266,7 @@ public void testMultipleDataDirs() throws Exception { + "-" + cluster.getClusterId(); assertEquals(name, cluster.getName()); - final String baseDir = getTempPath(name); + final String baseDir = MiniOzoneCluster.Builder.getTempPath(name); assertEquals(baseDir, cluster.getBaseDir()); diff --git a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java index 42fbe0b2e858..6c1a70923917 100644 --- a/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java +++ b/hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneCluster.java @@ -42,9 +42,6 @@ * Interface used for MiniOzoneClusters. */ public interface MiniOzoneCluster extends AutoCloseable { - String SYSPROP_TEST_DATA_DIR = "test.build.data"; - String DEFAULT_TEST_DATA_PATH = "target/test/data/"; - boolean WINDOWS = System.getProperty("os.name").startsWith("Windows"); /** * Returns the Builder to construct MiniOzoneCluster. @@ -252,29 +249,7 @@ default String getName() { } default String getBaseDir() { - return getTempPath(getName()); - } - - /** - * Get a temp path. This may or may not be relative; it depends on what the - * {@link #SYSPROP_TEST_DATA_DIR} is set to. If unset, it returns a path - * under the relative path {@link #DEFAULT_TEST_DATA_PATH} - * - * @param subpath sub path, with no leading "/" character - * @return a string to use in paths - */ - static String getTempPath(String subpath) { - String prop = WINDOWS ? DEFAULT_TEST_DATA_PATH - : System.getProperty(SYSPROP_TEST_DATA_DIR, DEFAULT_TEST_DATA_PATH); - - if (prop.isEmpty()) { - // corner case: property is there but empty - prop = DEFAULT_TEST_DATA_PATH; - } - if (!prop.endsWith("/")) { - prop = prop + "/"; - } - return prop + subpath; + return Builder.getTempPath(getName()); } /** @@ -287,6 +262,10 @@ abstract class Builder { protected static final int ACTIVE_SCMS_NOT_SET = -1; protected static final int DEFAULT_RATIS_RPC_TIMEOUT_SEC = 1; + private static final String SYSPROP_TEST_DATA_DIR = "test.build.data"; + private static final String DEFAULT_TEST_DATA_PATH = "target/test/data/"; + private static final boolean WINDOWS = System.getProperty("os.name").startsWith("Windows"); + protected OzoneConfiguration conf; protected String path; @@ -320,6 +299,28 @@ protected void prepareForNextBuild() { setClusterId(); } + /** + * Get a temp path. This may or may not be relative; it depends on what the + * {@link #SYSPROP_TEST_DATA_DIR} is set to. If unset, it returns a path + * under the relative path {@link #DEFAULT_TEST_DATA_PATH} + * + * @param subpath sub path, with no leading "/" character + * @return a string to use in paths + */ + protected static String getTempPath(String subpath) { + String prop = WINDOWS ? DEFAULT_TEST_DATA_PATH + : System.getProperty(SYSPROP_TEST_DATA_DIR, DEFAULT_TEST_DATA_PATH); + + if (prop.isEmpty()) { + // corner case: property is there but empty + prop = DEFAULT_TEST_DATA_PATH; + } + if (!prop.endsWith("/")) { + prop = prop + "/"; + } + return prop + subpath; + } + public Builder setSCMConfigurator(SCMConfigurator configurator) { this.scmConfigurator = configurator; return this;