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
This commit is contained in:
Arpit Agarwal 2019-05-18 02:47:19 -07:00 committed by Anu Engineer
parent 12c81610e0
commit 732133cb2a
4 changed files with 32 additions and 52 deletions

View File

@ -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 @@ private VolumeInfo(Builder b) throws IOException {
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() {

View File

@ -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.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 @@ long getAvailable() throws IOException {
}
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 @@ void saveScmUsed() {
* 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);
}
}

View File

@ -29,14 +29,12 @@
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 void testShutdown() throws Exception {
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();
}
}

View File

@ -35,7 +35,6 @@
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 void testShutdown() throws Exception {
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();
}
}