Addressed intitial set of review comments from Wei-Chiu

Author: S O'Donnell

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatProtobuf.java

@@ -368,10 +368,7 @@ public int compare(FileSummary.Section s1, FileSummary.Section s2) {
        * a particular step to be started for once.
        */
       Step currentStep = null;
-      boolean loadInParallel =
-          conf.getBoolean(DFSConfigKeys.DFS_IMAGE_PARALLEL_LOAD_KEY,
-              DFSConfigKeys.DFS_IMAGE_PARALLEL_LOAD_DEFAULT);
-      // TODO - check for compression and if enabled disable parallel
+      boolean loadInParallel = enableParallelSaveAndLoad(conf);
 
       ExecutorService executorService = null;
       ArrayList<FileSummary.Section> subSections =
@@ -556,6 +553,25 @@ private void loadErasureCodingSection(InputStream in)
     }
   }
 
+  private static boolean enableParallelSaveAndLoad(Configuration conf) {
+    boolean loadInParallel =
+        conf.getBoolean(DFSConfigKeys.DFS_IMAGE_PARALLEL_LOAD_KEY,
+            DFSConfigKeys.DFS_IMAGE_PARALLEL_LOAD_DEFAULT);
+    boolean compressionEnabled = conf.getBoolean(
+        DFSConfigKeys.DFS_IMAGE_COMPRESS_KEY,
+        DFSConfigKeys.DFS_IMAGE_COMPRESS_DEFAULT);
+
+    if (loadInParallel) {
+      if (compressionEnabled) {
+        LOG.warn("Parallel Image loading and saving is not supported when {}" +
+                " is set to true. Parallel will be disabled.",
+            DFSConfigKeys.DFS_IMAGE_COMPRESS_KEY);
+        loadInParallel = false;
+      }
+    }
+    return loadInParallel;
+  }
+
   public static final class Saver {
     public static final int CHECK_CANCEL_INTERVAL = 4096;
     private boolean writeSubSections = false;
@@ -690,30 +706,18 @@ long save(File file, FSImageCompression compression) throws IOException {
     }
 
     private void enableSubSectionsIfRequired() {
-      boolean parallelEnabled = conf.getBoolean(
-          DFSConfigKeys.DFS_IMAGE_PARALLEL_LOAD_KEY,
-          DFSConfigKeys.DFS_IMAGE_PARALLEL_LOAD_DEFAULT);
+      boolean parallelEnabled = enableParallelSaveAndLoad(conf);
       int inodeThreshold = conf.getInt(
           DFSConfigKeys.DFS_IMAGE_PARALLEL_INODE_THRESHOLD_KEY,
           DFSConfigKeys.DFS_IMAGE_PARALLEL_INODE_THRESHOLD_DEFAULT);
       int targetSections = conf.getInt(
           DFSConfigKeys.DFS_IMAGE_PARALLEL_TARGET_SECTIONS_KEY,
           DFSConfigKeys.DFS_IMAGE_PARALLEL_TARGET_SECTIONS_DEFAULT);
-      boolean compressionEnabled = conf.getBoolean(
-          DFSConfigKeys.DFS_IMAGE_COMPRESS_KEY,
-          DFSConfigKeys.DFS_IMAGE_COMPRESS_DEFAULT);
 
       if (parallelEnabled) {
-        if (compressionEnabled) {
-          LOG.warn("Parallel Image loading is not supported when {} is set to" +
-              " true. Parallel loading will be disabled.",
-              DFSConfigKeys.DFS_IMAGE_COMPRESS_KEY);
-          writeSubSections = false;
-          return;
-        }
         if (targetSections <= 0) {
           LOG.warn("{} is set to {}. It must be greater than zero. Setting to" +
-              "default of {}",
+              " default of {}",
               DFSConfigKeys.DFS_IMAGE_PARALLEL_TARGET_SECTIONS_KEY,
               targetSections,
               DFSConfigKeys.DFS_IMAGE_PARALLEL_TARGET_SECTIONS_DEFAULT);

hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml

@@ -1394,6 +1394,9 @@
         will be used for an image previously created with sub-sections.
         If the image contains sub-sections and this is set to false,
         parallel loading will not be used.
+        Parallel loading is not compatible with image compression,
+        so if dfs.image.compress is set to true this setting will be
+        ignored and no parallel loading will occur.
   </description>
 </property>