diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileReader.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileReader.java index 33acb91f837..f88aaeff075 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileReader.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileReader.java @@ -18,58 +18,87 @@ package org.apache.hadoop.hdfs.util; +import org.codehaus.jackson.JsonFactory; +import org.codehaus.jackson.map.JsonMappingException; +import org.codehaus.jackson.map.ObjectMapper; + +import java.io.EOFException; import java.io.FileInputStream; import java.io.InputStreamReader; import java.io.IOException; import java.io.Reader; - +import java.util.ArrayList; import java.util.Iterator; -import java.util.Set; -import java.util.HashSet; +import java.util.List; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; -import org.codehaus.jackson.JsonFactory; -import org.codehaus.jackson.map.ObjectMapper; - import org.apache.hadoop.hdfs.protocol.DatanodeAdminProperties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** - * Reader support for JSON based datanode configuration, an alternative + * Reader support for JSON-based datanode configuration, an alternative format * to the exclude/include files configuration. - * The JSON file format is the array of elements where each element + * The JSON file format defines the array of elements where each element * in the array describes the properties of a datanode. The properties of - * a datanode is defined in {@link DatanodeAdminProperties}. For example, + * a datanode is defined by {@link DatanodeAdminProperties}. For example, * - * {"hostName": "host1"} - * {"hostName": "host2", "port": 50, "upgradeDomain": "ud0"} - * {"hostName": "host3", "port": 0, "adminState": "DECOMMISSIONED"} + * [ + * {"hostName": "host1"}, + * {"hostName": "host2", "port": 50, "upgradeDomain": "ud0"}, + * {"hostName": "host3", "port": 0, "adminState": "DECOMMISSIONED"} + * ] */ @InterfaceAudience.LimitedPrivate({"HDFS"}) @InterfaceStability.Unstable public final class CombinedHostsFileReader { + + public static final Logger LOG = + LoggerFactory.getLogger(CombinedHostsFileReader.class); + private CombinedHostsFileReader() { } /** * Deserialize a set of DatanodeAdminProperties from a json file. - * @param hostsFile the input json file to read from. + * @param hostsFile the input json file to read from * @return the set of DatanodeAdminProperties * @throws IOException */ - public static Set + public static DatanodeAdminProperties[] readFile(final String hostsFile) throws IOException { - HashSet allDNs = new HashSet<>(); - ObjectMapper mapper = new ObjectMapper(); + DatanodeAdminProperties[] allDNs = new DatanodeAdminProperties[0]; + ObjectMapper objectMapper = new ObjectMapper(); + boolean tryOldFormat = false; try (Reader input = - new InputStreamReader(new FileInputStream(hostsFile), "UTF-8")) { - Iterator iterator = - mapper.readValues(new JsonFactory().createJsonParser(input), - DatanodeAdminProperties.class); - while (iterator.hasNext()) { - DatanodeAdminProperties properties = iterator.next(); - allDNs.add(properties); + new InputStreamReader(new FileInputStream(hostsFile), "UTF-8")) { + allDNs = objectMapper.readValue(input, DatanodeAdminProperties[].class); + } catch (JsonMappingException jme) { + // The old format doesn't have json top-level token to enclose the array. + // For backward compatibility, try parsing the old format. + tryOldFormat = true; + LOG.warn("{} has invalid JSON format." + + "Try the old format without top-level token defined.", hostsFile); + } catch(EOFException eof) { + LOG.warn("{} is empty.", hostsFile); + } + + if (tryOldFormat) { + JsonFactory jsonFactory = new JsonFactory(); + List all = new ArrayList<>(); + try (Reader input = + new InputStreamReader(new FileInputStream(hostsFile), "UTF-8")) { + Iterator iterator = + objectMapper.readValues(jsonFactory.createJsonParser(input), + DatanodeAdminProperties.class); + while (iterator.hasNext()) { + DatanodeAdminProperties properties = iterator.next(); + all.add(properties); + } } + allDNs = all.toArray(new DatanodeAdminProperties[all.size()]); } return allDNs; } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileWriter.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileWriter.java index ea70be2eb70..3d6d2e6cb75 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileWriter.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/util/CombinedHostsFileWriter.java @@ -25,22 +25,25 @@ import java.io.Writer; import java.util.Set; +import org.codehaus.jackson.map.ObjectMapper; + import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; -import org.codehaus.jackson.map.ObjectMapper; import org.apache.hadoop.hdfs.protocol.DatanodeAdminProperties; /** - * Writer support for JSON based datanode configuration, an alternative + * Writer support for JSON-based datanode configuration, an alternative format * to the exclude/include files configuration. - * The JSON file format is the array of elements where each element + * The JSON file format defines the array of elements where each element * in the array describes the properties of a datanode. The properties of - * a datanode is defined in {@link DatanodeAdminProperties}. For example, + * a datanode is defined by {@link DatanodeAdminProperties}. For example, * - * {"hostName": "host1"} - * {"hostName": "host2", "port": 50, "upgradeDomain": "ud0"} - * {"hostName": "host3", "port": 0, "adminState": "DECOMMISSIONED"} + * [ + * {"hostName": "host1"}, + * {"hostName": "host2", "port": 50, "upgradeDomain": "ud0"}, + * {"hostName": "host3", "port": 0, "adminState": "DECOMMISSIONED"} + * ] */ @InterfaceAudience.LimitedPrivate({"HDFS"}) @InterfaceStability.Unstable @@ -56,14 +59,11 @@ public final class CombinedHostsFileWriter { */ public static void writeFile(final String hostsFile, final Set allDNs) throws IOException { - StringBuilder configs = new StringBuilder(); + final ObjectMapper objectMapper = new ObjectMapper(); + try (Writer output = new OutputStreamWriter(new FileOutputStream(hostsFile), "UTF-8")) { - for (DatanodeAdminProperties datanodeAdminProperties: allDNs) { - ObjectMapper mapper = new ObjectMapper(); - configs.append(mapper.writeValueAsString(datanodeAdminProperties)); - } - output.write(configs.toString()); + objectMapper.writeValue(output, allDNs); } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CombinedHostFileManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CombinedHostFileManager.java index 3e913b93a25..27117936b68 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CombinedHostFileManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CombinedHostFileManager.java @@ -39,7 +39,6 @@ import java.net.InetSocketAddress; import java.util.Collection; import java.util.Iterator; import java.util.Map; -import java.util.Set; import com.google.common.base.Predicate; @@ -194,7 +193,7 @@ public class CombinedHostFileManager extends HostConfigManager { } private void refresh(final String hostsFile) throws IOException { HostProperties hostProps = new HostProperties(); - Set all = + DatanodeAdminProperties[] all = CombinedHostsFileReader.readFile(hostsFile); for(DatanodeAdminProperties properties : all) { InetSocketAddress addr = parseEntry(hostsFile, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/util/TestCombinedHostsFileReader.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/util/TestCombinedHostsFileReader.java index c3946e412b2..3c92a86971d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/util/TestCombinedHostsFileReader.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/util/TestCombinedHostsFileReader.java @@ -20,28 +20,29 @@ package org.apache.hadoop.hdfs.util; import java.io.File; import java.io.FileWriter; -import java.util.Set; - import org.apache.hadoop.hdfs.protocol.DatanodeAdminProperties; +import org.apache.hadoop.test.GenericTestUtils; import org.junit.Before; import org.junit.After; import org.junit.Test; import static org.junit.Assert.assertEquals; -/* - * Test for JSON based HostsFileReader +/** + * Test for JSON based HostsFileReader. */ public class TestCombinedHostsFileReader { // Using /test/build/data/tmp directory to store temporary files - static final String HOSTS_TEST_DIR = new File(System.getProperty( - "test.build.data", "/tmp")).getAbsolutePath(); - File NEW_FILE = new File(HOSTS_TEST_DIR, "dfs.hosts.new.json"); + static final String HOSTSTESTDIR = GenericTestUtils.getTestDir() + .getAbsolutePath(); + private final File newFile = new File(HOSTSTESTDIR, "dfs.hosts.new.json"); - static final String TEST_CACHE_DATA_DIR = + static final String TESTCACHEDATADIR = System.getProperty("test.cache.data", "build/test/cache"); - File EXISTING_FILE = new File(TEST_CACHE_DATA_DIR, "dfs.hosts.json"); + private final File jsonFile = new File(TESTCACHEDATADIR, "dfs.hosts.json"); + private final File legacyFile = + new File(TESTCACHEDATADIR, "legacy.dfs.hosts.json"); @Before public void setUp() throws Exception { @@ -50,18 +51,28 @@ public class TestCombinedHostsFileReader { @After public void tearDown() throws Exception { // Delete test file after running tests - NEW_FILE.delete(); + newFile.delete(); } /* - * Load the existing test json file + * Load the legacy test json file + */ + @Test + public void testLoadLegacyJsonFile() throws Exception { + DatanodeAdminProperties[] all = + CombinedHostsFileReader.readFile(legacyFile.getAbsolutePath()); + assertEquals(5, all.length); + } + + /* + * Load the test json file */ @Test public void testLoadExistingJsonFile() throws Exception { - Set all = - CombinedHostsFileReader.readFile(EXISTING_FILE.getAbsolutePath()); - assertEquals(5, all.size()); + DatanodeAdminProperties[] all = + CombinedHostsFileReader.readFile(jsonFile.getAbsolutePath()); + assertEquals(5, all.length); } /* @@ -69,11 +80,11 @@ public class TestCombinedHostsFileReader { */ @Test public void testEmptyCombinedHostsFileReader() throws Exception { - FileWriter hosts = new FileWriter(NEW_FILE); + FileWriter hosts = new FileWriter(newFile); hosts.write(""); hosts.close(); - Set all = - CombinedHostsFileReader.readFile(NEW_FILE.getAbsolutePath()); - assertEquals(0, all.size()); + DatanodeAdminProperties[] all = + CombinedHostsFileReader.readFile(newFile.getAbsolutePath()); + assertEquals(0, all.length); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/dfs.hosts.json b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/dfs.hosts.json index 64fca48dbff..18a2fa2822f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/dfs.hosts.json +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/dfs.hosts.json @@ -1,5 +1,7 @@ -{"hostName": "host1"} -{"hostName": "host2", "upgradeDomain": "ud0"} -{"hostName": "host3", "adminState": "DECOMMISSIONED"} -{"hostName": "host4", "upgradeDomain": "ud2", "adminState": "DECOMMISSIONED"} -{"hostName": "host5", "port": 8090} +[ + {"hostName": "host1"}, + {"hostName": "host2", "upgradeDomain": "ud0"}, + {"hostName": "host3", "adminState": "DECOMMISSIONED"}, + {"hostName": "host4", "upgradeDomain": "ud2", "adminState": "DECOMMISSIONED"}, + {"hostName": "host5", "port": 8090} +] diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/legacy.dfs.hosts.json b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/legacy.dfs.hosts.json new file mode 100644 index 00000000000..64fca48dbff --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/legacy.dfs.hosts.json @@ -0,0 +1,5 @@ +{"hostName": "host1"} +{"hostName": "host2", "upgradeDomain": "ud0"} +{"hostName": "host3", "adminState": "DECOMMISSIONED"} +{"hostName": "host4", "upgradeDomain": "ud2", "adminState": "DECOMMISSIONED"} +{"hostName": "host5", "port": 8090}