Skip to content

Commit 1d64b37

Browse files
peterxclipeterxcli
authored
HDDS-12380. Fix spotbugs warnings in hdds-container-service (#7958)
1 parent 5c2b8f6 commit 1d64b37

14 files changed

Lines changed: 67 additions & 109 deletions

File tree

‎hadoop-hdds/container-service/dev-support/findbugsExcludeFile.xml‎

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -15,70 +15,4 @@
1515
limitations under the License.
1616
-->
1717
<FindBugsFilter>
18-
19-
<!-- Test -->
20-
<Match>
21-
<Class name="org.apache.hadoop.ozone.container.common.TestContainerCache" />
22-
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
23-
</Match>
24-
<Match>
25-
<Class name="org.apache.hadoop.ozone.container.common.TestDatanodeStateMachine" />
26-
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
27-
</Match>
28-
<Match>
29-
<Class name="org.apache.hadoop.ozone.container.common.TestDatanodeStateMachine" />
30-
<Bug pattern="REC_CATCH_EXCEPTION" />
31-
</Match>
32-
<Match>
33-
<Class name="org.apache.hadoop.ozone.container.common.impl.TestContainerDataYaml" />
34-
<Bug pattern="DLS_DEAD_LOCAL_STORE" />
35-
</Match>
36-
<Match>
37-
<Class name="org.apache.hadoop.ozone.container.common.impl.TestContainerDataYaml" />
38-
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
39-
</Match>
40-
<Match>
41-
<Class name="org.apache.hadoop.ozone.container.common.impl.TestContainerDeletionChoosingPolicy" />
42-
<Bug pattern="UC_USELESS_OBJECT" />
43-
</Match>
44-
<Match>
45-
<Class name="org.apache.hadoop.ozone.container.common.impl.TestContainerPersistence" />
46-
<Bug pattern="DLS_DEAD_LOCAL_STORE" />
47-
</Match>
48-
<Match>
49-
<Class name="org.apache.hadoop.ozone.container.common.interfaces.TestHandler" />
50-
<Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
51-
</Match>
52-
<Match>
53-
<Class name="org.apache.hadoop.ozone.container.common.statemachine.TestStateContext" />
54-
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
55-
</Match>
56-
<Match>
57-
<Class name="org.apache.hadoop.ozone.container.common.statemachine.commandhandler.TestDeleteBlocksCommandHandler" />
58-
<Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
59-
</Match>
60-
<Match>
61-
<Class name="org.apache.hadoop.ozone.container.common.volume.TestVolumeSet" />
62-
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
63-
</Match>
64-
<Match>
65-
<Class name="org.apache.hadoop.ozone.container.common.volume.TestVolumeSet" />
66-
<Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
67-
</Match>
68-
<Match>
69-
<Class name="org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainerMarkUnhealthy" />
70-
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
71-
</Match>
72-
<Match>
73-
<Class name="org.apache.hadoop.ozone.container.keyvalue.TestTarContainerPacker" />
74-
<Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE" />
75-
</Match>
76-
<Match>
77-
<Class name="org.apache.hadoop.ozone.container.ozoneimpl.TestBackgroundContainerDataScanner"/>
78-
<Bug pattern="RU_INVOKE_RUN, RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
79-
</Match>
80-
<Match>
81-
<Class name="org.apache.hadoop.ozone.container.ozoneimpl.TestBackgroundContainerMetadataScanner"/>
82-
<Bug pattern="RU_INVOKE_RUN, RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
83-
</Match>
8418
</FindBugsFilter>

‎hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/AbstractBackgroundContainerScanner.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,26 @@
2929
/**
3030
* Base class for scheduled scanners on a Datanode.
3131
*/
32-
public abstract class AbstractBackgroundContainerScanner extends Thread {
32+
public abstract class AbstractBackgroundContainerScanner implements Runnable {
3333
public static final Logger LOG =
3434
LoggerFactory.getLogger(AbstractBackgroundContainerScanner.class);
3535

3636
private final long dataScanInterval;
37-
37+
private final Thread scannerThread;
3838
private final AtomicBoolean stopping;
3939
private final AtomicBoolean pausing = new AtomicBoolean();
4040

4141
public AbstractBackgroundContainerScanner(String name,
4242
long dataScanInterval) {
4343
this.dataScanInterval = dataScanInterval;
4444
this.stopping = new AtomicBoolean(false);
45-
setName(name);
46-
setDaemon(true);
45+
46+
this.scannerThread = new Thread(this, name);
47+
this.scannerThread.setDaemon(true);
48+
}
49+
50+
public void start() {
51+
scannerThread.start();
4752
}
4853

4954
@Override
@@ -141,16 +146,20 @@ public final void handleRemainingSleep(long remainingSleep) {
141146
*/
142147
public synchronized void shutdown() {
143148
if (stopping.compareAndSet(false, true)) {
144-
this.interrupt();
149+
scannerThread.interrupt();
145150
try {
146-
this.join();
151+
scannerThread.join();
147152
} catch (InterruptedException ex) {
148153
LOG.warn("Unexpected exception while stopping data scanner.", ex);
149154
Thread.currentThread().interrupt();
150155
}
151156
}
152157
}
153158

159+
public boolean isAlive() {
160+
return scannerThread.isAlive();
161+
}
162+
154163
public void pause() {
155164
pausing.getAndSet(true);
156165
}

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/SCMTestUtils.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package org.apache.hadoop.ozone.container.common;
1919

20+
import static org.junit.jupiter.api.Assertions.assertTrue;
2021
import static org.mockito.Mockito.mock;
2122

2223
import com.google.protobuf.BlockingService;
@@ -122,12 +123,18 @@ public static InetSocketAddress getReuseableAddress() throws IOException {
122123

123124
public static OzoneConfiguration getConf(File testDir) {
124125
OzoneConfiguration conf = new OzoneConfiguration();
126+
File datanodeDir = new File(testDir, "datanode");
127+
File metadataDir = new File(testDir, "metadata");
128+
File datanodeIdDir = new File(testDir, "datanodeID");
129+
assertTrue(datanodeDir.mkdirs());
130+
assertTrue(metadataDir.mkdirs());
131+
assertTrue(datanodeIdDir.mkdirs());
125132
conf.set(ScmConfigKeys.HDDS_DATANODE_DIR_KEY,
126-
new File(testDir, "datanode").getAbsolutePath());
133+
datanodeDir.getAbsolutePath());
127134
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS,
128-
new File(testDir, "metadata").getAbsolutePath());
135+
metadataDir.getAbsolutePath());
129136
conf.set(ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR,
130-
new File(testDir, "datanodeID").getAbsolutePath());
137+
datanodeIdDir.getAbsolutePath());
131138
conf.setClass(SpaceUsageCheckFactory.Conf.configKeyForClassName(),
132139
MockSpaceUsageCheckFactory.None.class,
133140
SpaceUsageCheckFactory.class);

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestContainerCache.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static org.junit.jupiter.api.Assertions.assertNotNull;
2323
import static org.junit.jupiter.api.Assertions.assertNull;
2424
import static org.junit.jupiter.api.Assertions.assertThrows;
25+
import static org.junit.jupiter.api.Assertions.assertTrue;
2526
import static org.junit.jupiter.api.Assertions.fail;
2627

2728
import java.io.File;
@@ -31,6 +32,7 @@
3132
import java.util.concurrent.ExecutorService;
3233
import java.util.concurrent.Executors;
3334
import java.util.concurrent.Future;
35+
import org.apache.commons.io.FileUtils;
3436
import org.apache.hadoop.fs.FileSystemTestHelper;
3537
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
3638
import org.apache.hadoop.ozone.OzoneConfigKeys;
@@ -63,7 +65,7 @@ private void createContainerDB(OzoneConfiguration conf, File dbFile)
6365
@Test
6466
public void testContainerCacheEviction() throws Exception {
6567
File root = new File(testRoot);
66-
root.mkdirs();
68+
assertTrue(root.mkdirs());
6769

6870
OzoneConfiguration conf = new OzoneConfiguration();
6971
conf.setInt(OzoneConfigKeys.OZONE_CONTAINER_CACHE_SIZE, 2);
@@ -139,13 +141,14 @@ public void testContainerCacheEviction() throws Exception {
139141
db4.close();
140142
db5.close();
141143
});
144+
145+
FileUtils.deleteDirectory(root);
142146
}
143147

144148
@Test
145149
void testConcurrentDBGet() throws Exception {
146150
File root = new File(testRoot);
147-
root.mkdirs();
148-
root.deleteOnExit();
151+
assertTrue(root.mkdirs());
149152

150153
OzoneConfiguration conf = new OzoneConfiguration();
151154
conf.setInt(OzoneConfigKeys.OZONE_CONTAINER_CACHE_SIZE, 2);
@@ -180,12 +183,13 @@ void testConcurrentDBGet() throws Exception {
180183
db.close();
181184
assertEquals(1, cache.size());
182185
db.cleanup();
186+
FileUtils.deleteDirectory(root);
183187
}
184188

185189
@Test
186190
public void testUnderlyingDBzIsClosed() throws Exception {
187191
File root = new File(testRoot);
188-
root.mkdirs();
192+
assertTrue(root.mkdirs());
189193

190194
OzoneConfiguration conf = new OzoneConfiguration();
191195
conf.setInt(OzoneConfigKeys.OZONE_CONTAINER_CACHE_SIZE, 2);
@@ -217,5 +221,6 @@ public void testUnderlyingDBzIsClosed() throws Exception {
217221
db3.close();
218222
db4.close();
219223
cache.clear();
224+
FileUtils.deleteDirectory(root);
220225
}
221226
}

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ public void testDatanodeStateContext() throws IOException,
195195
File idPath = new File(
196196
conf.get(ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR),
197197
OzoneConsts.OZONE_SCM_DATANODE_ID_FILE_DEFAULT);
198-
idPath.delete();
198+
assertTrue(idPath.createNewFile());
199+
assertTrue(idPath.delete());
199200
DatanodeDetails datanodeDetails = getNewDatanodeDetails();
200201
DatanodeDetails.Port port = DatanodeDetails.newStandalonePort(
201202
OzoneConfigKeys.HDDS_CONTAINER_IPC_PORT_DEFAULT);
@@ -317,11 +318,11 @@ public void testDatanodeStateContext() throws IOException,
317318

318319
@Test
319320
public void testDatanodeStateMachineWithIdWriteFail() throws Exception {
320-
321321
File idPath = new File(
322322
conf.get(ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR),
323323
OzoneConsts.OZONE_SCM_DATANODE_ID_FILE_DEFAULT);
324-
idPath.delete();
324+
assertTrue(idPath.createNewFile());
325+
assertTrue(idPath.delete());
325326
DatanodeDetails datanodeDetails = getNewDatanodeDetails();
326327
DatanodeDetails.Port port = DatanodeDetails.newStandalonePort(
327328
OzoneConfigKeys.HDDS_CONTAINER_IPC_PORT_DEFAULT);
@@ -340,8 +341,7 @@ public void testDatanodeStateMachineWithIdWriteFail() throws Exception {
340341

341342
//Set the idPath to read only, state machine will fail to write
342343
// datanodeId file and set the state to shutdown.
343-
idPath.getParentFile().mkdirs();
344-
idPath.getParentFile().setReadOnly();
344+
assertTrue(idPath.getParentFile().setReadOnly());
345345

346346
task.execute(executorService);
347347
DatanodeStateMachine.DatanodeStates newState =
@@ -398,7 +398,7 @@ public void testDatanodeStateMachineWithInvalidConfiguration()
398398
task.await(2, TimeUnit.SECONDS);
399399
assertEquals(DatanodeStateMachine.DatanodeStates.SHUTDOWN,
400400
newState);
401-
} catch (Exception e) {
401+
} catch (IOException | InterruptedException | TimeoutException | ExecutionException e) {
402402
fail("Unexpected exception found");
403403
}
404404
});

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private void setLayoutVersion(ContainerLayoutVersion layoutVersion) {
7070
*/
7171
private File createContainerFile(long containerID, int replicaIndex)
7272
throws IOException {
73-
new File(testRoot).mkdirs();
73+
assertTrue(new File(testRoot).mkdirs());
7474

7575
String containerPath = containerID + ".container";
7676

@@ -167,6 +167,8 @@ public void testCreateContainerFile(ContainerLayoutVersion layout)
167167
kvData.lastDataScanTime().get().toEpochMilli());
168168
assertEquals(SCAN_TIME.toEpochMilli(),
169169
kvData.getDataScanTimestamp().longValue());
170+
171+
cleanup();
170172
}
171173

172174
@ContainerLayoutTestInfo.ContainerTest

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDeletionChoosingPolicy.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.util.ArrayList;
3030
import java.util.Collections;
3131
import java.util.HashMap;
32-
import java.util.LinkedList;
3332
import java.util.List;
3433
import java.util.Map;
3534
import java.util.Random;
@@ -39,7 +38,6 @@
3938
import org.apache.commons.lang3.RandomUtils;
4039
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
4140
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
42-
import org.apache.hadoop.hdfs.server.datanode.StorageLocation;
4341
import org.apache.hadoop.ozone.container.ContainerTestHelper;
4442
import org.apache.hadoop.ozone.container.common.impl.BlockDeletingService.ContainerBlockInfo;
4543
import org.apache.hadoop.ozone.container.common.interfaces.ContainerDeletionChoosingPolicy;
@@ -83,8 +81,6 @@ public void testRandomChoosingPolicy(ContainerLayoutVersion layout)
8381
conf.set(
8482
ScmConfigKeys.OZONE_SCM_KEY_VALUE_CONTAINER_DELETION_CHOOSING_POLICY,
8583
RandomContainerDeletionChoosingPolicy.class.getName());
86-
List<StorageLocation> pathLists = new LinkedList<>();
87-
pathLists.add(StorageLocation.parse(containerDir.getAbsolutePath()));
8884
containerSet = new ContainerSet(1000);
8985

9086
int numContainers = 10;
@@ -146,8 +142,6 @@ public void testTopNOrderedChoosingPolicy(ContainerLayoutVersion layout)
146142
conf.set(
147143
ScmConfigKeys.OZONE_SCM_KEY_VALUE_CONTAINER_DELETION_CHOOSING_POLICY,
148144
TopNOrderedContainerDeletionChoosingPolicy.class.getName());
149-
List<StorageLocation> pathLists = new LinkedList<>();
150-
pathLists.add(StorageLocation.parse(containerDir.getAbsolutePath()));
151145
containerSet = new ContainerSet(1000);
152146

153147
int numContainers = 10;

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import java.io.IOException;
3939
import java.nio.file.Files;
4040
import java.nio.file.Path;
41-
import java.nio.file.Paths;
4241
import java.security.NoSuchAlgorithmException;
4342
import java.util.ArrayList;
4443
import java.util.Arrays;
@@ -660,7 +659,6 @@ public void testWritReadManyChunks(ContainerTestVersionInfo versionInfo)
660659
KeyValueContainerData cNewData =
661660
(KeyValueContainerData) container.getContainerData();
662661
assertNotNull(cNewData);
663-
Path dataDir = Paths.get(cNewData.getChunksPath());
664662

665663
// Read chunk via file system and verify.
666664
Checksum checksum = new Checksum(ChecksumType.CRC32, 1024 * 1024);

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/TestStateContext.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.concurrent.CompletableFuture;
4545
import java.util.concurrent.ExecutorService;
4646
import java.util.concurrent.Executors;
47+
import java.util.concurrent.Future;
4748
import java.util.concurrent.TimeUnit;
4849
import java.util.concurrent.TimeoutException;
4950
import java.util.concurrent.atomic.AtomicBoolean;
@@ -572,9 +573,11 @@ public void testIsThreadPoolAvailable() throws Exception {
572573

573574
// task num greater than pool size
574575
for (int i = 0; i < threadPoolSize; i++) {
575-
executorService.submit((Callable<String>) futureOne::get);
576+
Future<String> future = executorService.submit((Callable<String>) futureOne::get);
577+
assertFalse(future.isDone());
576578
}
577-
executorService.submit((Callable<String>) futureTwo::get);
579+
Future<String> future = executorService.submit((Callable<String>) futureTwo::get);
580+
assertFalse(future.isDone());
578581

579582
assertFalse(stateContext.isThreadPoolAvailable(executorService));
580583

@@ -592,9 +595,11 @@ public void doesNotAwaitWithoutExecute() throws Exception {
592595
final AtomicInteger awaited = new AtomicInteger();
593596

594597
ExecutorService executorService = Executors.newFixedThreadPool(1);
595-
CompletableFuture<String> future = new CompletableFuture<>();
596-
executorService.submit((Callable<String>) future::get);
597-
executorService.submit((Callable<String>) future::get);
598+
CompletableFuture<String> task = new CompletableFuture<>();
599+
Future<String> future = executorService.submit((Callable<String>) task::get);
600+
assertFalse(future.isDone());
601+
future = executorService.submit((Callable<String>) task::get);
602+
assertFalse(future.isDone());
598603

599604
StateContext subject = new StateContext(new OzoneConfiguration(),
600605
DatanodeStates.INIT, mock(DatanodeStateMachine.class), "") {
@@ -631,7 +636,7 @@ public DatanodeStates await(long time, TimeUnit timeUnit) {
631636
assertEquals(0, awaited.get());
632637
assertEquals(0, executed.get());
633638

634-
future.complete("any");
639+
task.complete("any");
635640
LambdaTestUtils.await(1000, 100, () ->
636641
subject.isThreadPoolAvailable(executorService));
637642

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ public void testVolumeInInconsistentState() throws Exception {
185185
// Create the root volume dir and create a sub-directory within it.
186186
File newVolume = new File(volume3, HDDS_VOLUME_DIR);
187187
System.out.println("new volume root: " + newVolume);
188-
newVolume.mkdirs();
188+
assertTrue(newVolume.mkdirs());
189189
assertTrue(newVolume.exists(), "Failed to create new volume root");
190190
File dataDir = new File(newVolume, "chunks");
191-
dataDir.mkdirs();
191+
assertTrue(dataDir.mkdirs());
192192
assertTrue(dataDir.exists());
193193

194194
// The new volume is in an inconsistent state as the root dir is

0 commit comments

Comments
 (0)