This is an automated email from the ASF dual-hosted git repository.
aengineer pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new 732133c HDDS-1422. Exception during DataNode shutdown. Contributed by
Arpit A… (#725)
732133c is described below
commit 732133cb2a8f8667da1ae955d5ce73eef1a0436e
Author: Arpit Agarwal <a...@users.noreply.github.com>
AuthorDate: Sat May 18 02:47:19 2019 -0700
HDDS-1422. Exception during DataNode shutdown. Contributed by Arpit A…
(#725)
* HDDS-1422. Exception during DataNode shutdown. Contributed by Arpit
Agarwal.
Change-Id: I6db6bdd19839a45e5341ed7e745cd38f68af8378
* Suppress spurious findbugs warning.
* Remove log file that got committed in error
---
.../ozone/container/common/volume/VolumeInfo.java | 20 ++-----------
.../ozone/container/common/volume/VolumeUsage.java | 34 +++++++++++++++-------
.../container/common/volume/TestHddsVolume.java | 15 ++--------
.../container/common/volume/TestVolumeSet.java | 15 ++--------
4 files changed, 32 insertions(+), 52 deletions(-)
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java
index 0d69898..31f83ec 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java
@@ -39,7 +39,8 @@ public final class VolumeInfo {
private final StorageType storageType;
// Space usage calculator
- private VolumeUsage usage;
+ private final VolumeUsage usage;
+
// Capacity configured. This is useful when we want to
// limit the visible capacity for tests. If negative, then we just
// query from the filesystem.
@@ -97,36 +98,21 @@ public final class VolumeInfo {
public long getCapacity() throws IOException {
if (configuredCapacity < 0) {
- if (usage == null) {
- throw new IOException("Volume Usage thread is not running. This error"
+
- " is usually seen during DataNode shutdown.");
- }
return usage.getCapacity();
}
return configuredCapacity;
}
public long getAvailable() throws IOException {
- if (usage == null) {
- throw new IOException("Volume Usage thread is not running. This error " +
- "is usually seen during DataNode shutdown.");
- }
return usage.getAvailable();
}
public long getScmUsed() throws IOException {
- if (usage == null) {
- throw new IOException("Volume Usage thread is not running. This error " +
- "is usually seen during DataNode shutdown.");
- }
return usage.getScmUsed();
}
protected void shutdownUsageThread() {
- if (usage != null) {
- usage.shutdown();
- }
- usage = null;
+ usage.shutdown();
}
public String getRootDir() {
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java
index 2c7563e..693bcb5 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java
@@ -19,6 +19,7 @@
package org.apache.hadoop.ozone.container.common.volume;
import com.google.common.annotations.VisibleForTesting;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CachingGetSpaceUsed;
import org.apache.hadoop.fs.DF;
@@ -35,6 +36,7 @@ import java.io.IOException;
import java.io.OutputStreamWriter;
import java.nio.charset.StandardCharsets;
import java.util.Scanner;
+import java.util.concurrent.atomic.AtomicReference;
/**
* Class that wraps the space df of the Datanode Volumes used by SCM
@@ -46,7 +48,8 @@ public class VolumeUsage {
private final File rootDir;
private final DF df;
private final File scmUsedFile;
- private GetSpaceUsed scmUsage;
+ private AtomicReference<GetSpaceUsed> scmUsage;
+ private boolean shutdownComplete;
private static final String DU_CACHE_FILE = "scmUsed";
private volatile boolean scmUsedSaved = false;
@@ -65,10 +68,11 @@ public class VolumeUsage {
void startScmUsageThread(Configuration conf) throws IOException {
// get SCM specific df
- this.scmUsage = new CachingGetSpaceUsed.Builder().setPath(rootDir)
- .setConf(conf)
- .setInitialUsed(loadScmUsed())
- .build();
+ scmUsage = new AtomicReference<>(
+ new CachingGetSpaceUsed.Builder().setPath(rootDir)
+ .setConf(conf)
+ .setInitialUsed(loadScmUsed())
+ .build());
}
long getCapacity() {
@@ -89,14 +93,18 @@ public class VolumeUsage {
}
long getScmUsed() throws IOException{
- return scmUsage.getUsed();
+ return scmUsage.get().getUsed();
}
- public void shutdown() {
- saveScmUsed();
+ public synchronized void shutdown() {
+ if (!shutdownComplete) {
+ saveScmUsed();
- if (scmUsage instanceof CachingGetSpaceUsed) {
- IOUtils.cleanupWithLogger(null, ((CachingGetSpaceUsed) scmUsage));
+ if (scmUsage.get() instanceof CachingGetSpaceUsed) {
+ IOUtils.cleanupWithLogger(
+ null, ((CachingGetSpaceUsed) scmUsage.get()));
+ }
+ shutdownComplete = true;
}
}
@@ -175,7 +183,11 @@ public class VolumeUsage {
* Only for testing. Do not use otherwise.
*/
@VisibleForTesting
+ @SuppressFBWarnings(
+ value = "IS2_INCONSISTENT_SYNC",
+ justification = "scmUsage is an AtomicReference. No additional " +
+ "synchronization is needed.")
public void setScmUsageForTesting(GetSpaceUsed scmUsageForTest) {
- this.scmUsage = scmUsageForTest;
+ scmUsage.set(scmUsageForTest);
}
}
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java
index 0e58e69..fb2f29b 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestHddsVolume.java
@@ -29,14 +29,12 @@ import org.junit.rules.TemporaryFolder;
import org.mockito.Mockito;
import java.io.File;
-import java.io.IOException;
import java.util.Properties;
import java.util.UUID;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
/**
* Unit tests for {@link HddsVolume}.
@@ -134,15 +132,8 @@ public class TestHddsVolume {
assertTrue("scmUsed cache file should be saved on shutdown",
scmUsedFile.exists());
- try {
- // Volume.getAvailable() should fail with IOException
- // as usage thread is shutdown.
- volume.getAvailable();
- fail("HddsVolume#shutdown test failed");
- } catch (Exception ex) {
- assertTrue(ex instanceof IOException);
- assertTrue(ex.getMessage().contains(
- "Volume Usage thread is not running."));
- }
+ // Volume.getAvailable() should succeed even when usage thread
+ // is shutdown.
+ volume.getAvailable();
}
}
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java
index c50ec78..f97ad4e 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java
@@ -35,7 +35,6 @@ import static
org.apache.hadoop.ozone.container.common.volume.HddsVolume
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
import org.junit.After;
import org.junit.Assert;
@@ -213,18 +212,10 @@ public class TestVolumeSet {
volumeSet.shutdown();
- // Verify that the volumes are shutdown and the volumeUsage is set to null.
+ // Verify that volume usage can be queried during shutdown.
for (HddsVolume volume : volumesList) {
- Assert.assertNull(volume.getVolumeInfo().getUsageForTesting());
- try {
- // getAvailable() should throw null pointer exception as usage is null.
- volume.getAvailable();
- fail("Volume shutdown failed.");
- } catch (IOException ex) {
- // Do Nothing. Exception is expected.
- assertTrue(ex.getMessage().contains(
- "Volume Usage thread is not running."));
- }
+ Assert.assertNotNull(volume.getVolumeInfo().getUsageForTesting());
+ volume.getAvailable();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
