HDFS-14810. Review FSNameSystem editlog sync. Contributed by Xiaoqiao He.

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

@@ -2114,9 +2114,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       logAuditEvent(false, operationName, Arrays.toString(srcs),
           target, stat);
       throw ace;
-    } finally {
-      getEditLog().logSync();
     }
+    getEditLog().logSync();
     logAuditEvent(true, operationName, Arrays.toString(srcs), target, stat);
   }
 
@@ -3076,7 +3075,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     boolean success = ret.success;
     if (success) {
       getEditLog().logSync();
-      logAuditEvent(success, operationName, src, dst, ret.auditStat);
+      logAuditEvent(true, operationName, src, dst, ret.auditStat);
     }
     return success;
   }
@@ -3259,11 +3258,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     final String operationName = "isFileClosed";
     checkOperation(OperationCategory.READ);
     final FSPermissionChecker pc = getPermissionChecker();
+    boolean success = false;
     try {
       readLock();
       try {
         checkOperation(OperationCategory.READ);
-        return FSDirStatAndListingOp.isFileClosed(dir, pc, src);
+        success = FSDirStatAndListingOp.isFileClosed(dir, pc, src);
       } finally {
         readUnlock(operationName);
       }
@@ -3271,6 +3271,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       logAuditEvent(false, operationName, src);
       throw e;
     }
+    if (success) {
+      logAuditEvent(true, operationName, src);
+    }
+    return success;
   }
 
   /**
@@ -3398,9 +3402,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     } catch (AccessControlException ace) {
       logAuditEvent(false, operationName, src);
       throw ace;
-    } finally {
-      getEditLog().logSync();
     }
+    getEditLog().logSync();
     logAuditEvent(true, operationName, src);
   }
 
@@ -5745,7 +5748,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
   Token<DelegationTokenIdentifier> getDelegationToken(Text renewer)
       throws IOException {
     final String operationName = "getDelegationToken";
-    final boolean success;
     final String tokenId;
     Token<DelegationTokenIdentifier> token;
     checkOperation(OperationCategory.WRITE);
@@ -5776,12 +5778,11 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       long expiryTime = dtSecretManager.getTokenExpiryTime(dtId);
       getEditLog().logGetDelegationToken(dtId, expiryTime);
       tokenId = dtId.toStringStable();
-      success = true;
     } finally {
       writeUnlock(operationName);
     }
     getEditLog().logSync();
-    logAuditEvent(success, operationName, tokenId);
+    logAuditEvent(true, operationName, tokenId);
     return token;
   }
 
@@ -6555,19 +6556,17 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
   void allowSnapshot(String path) throws IOException {
     checkOperation(OperationCategory.WRITE);
     final String operationName = "allowSnapshot";
-    boolean success = false;
     checkSuperuserPrivilege(operationName);
     writeLock();
     try {
       checkOperation(OperationCategory.WRITE);
       checkNameNodeSafeMode("Cannot allow snapshot for " + path);
       FSDirSnapshotOp.allowSnapshot(dir, snapshotManager, path);
-      success = true;
     } finally {
       writeUnlock(operationName);
     }
     getEditLog().logSync();
-    logAuditEvent(success, operationName, path, null, null);
+    logAuditEvent(true, operationName, path, null, null);
   }
   
   /** Disallow snapshot on a directory. */
@@ -6609,8 +6608,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         writeUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, snapshotRoot,
-          snapshotPath, null);
+      logAuditEvent(false, operationName, snapshotRoot);
       throw ace;
     }
     getEditLog().logSync();
@@ -7124,15 +7122,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         writeUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, null,
-          null, null);
+      logAuditEvent(false, operationName, null);
       throw ace;
-    } finally {
-      getEditLog().logSync();
     }
+    getEditLog().logSync();
     effectiveDirectiveStr = effectiveDirective.toString();
-    logAuditEvent(true, operationName, effectiveDirectiveStr,
-        null, null);
+    logAuditEvent(true, operationName, effectiveDirectiveStr);
     return effectiveDirective.getId();
   }
 
@@ -7158,9 +7153,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       logAuditEvent(false, operationName, idStr,
           directive.toString(), null);
       throw ace;
-    } finally {
-      getEditLog().logSync();
     }
+    getEditLog().logSync();
     logAuditEvent(true, operationName, idStr,
         directive.toString(), null);
   }
@@ -7183,8 +7177,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       logAuditEvent(false, operationName, idStr, null, null);
       throw ace;
     }
-    logAuditEvent(true, operationName, idStr, null, null);
     getEditLog().logSync();
+    logAuditEvent(true, operationName, idStr, null, null);
   }
 
   BatchedListEntries<CacheDirectiveEntry> listCacheDirectives(
@@ -7203,12 +7197,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         readUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, filter.toString(), null,
-          null);
+      logAuditEvent(false, operationName, filter.toString());
       throw ace;
     }
-    logAuditEvent(true, operationName, filter.toString(), null,
-        null);
+    logAuditEvent(true, operationName, filter.toString());
     return results;
   }
 
@@ -7230,11 +7222,11 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         writeUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, poolInfoStr, null, null);
+      logAuditEvent(false, operationName, poolInfoStr);
       throw ace;
     }
-    logAuditEvent(true, operationName, poolInfoStr, null, null);
     getEditLog().logSync();
+    logAuditEvent(true, operationName, poolInfoStr);
   }
 
   void modifyCachePool(CachePoolInfo req, boolean logRetryCache)
@@ -7258,10 +7250,9 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
           req == null ? null : req.toString(), null);
       throw ace;
     }
+    getEditLog().logSync();
     logAuditEvent(true, operationName, poolNameStr,
         req == null ? null : req.toString(), null);
-
-    getEditLog().logSync();
   }
 
   void removeCachePool(String cachePoolName, boolean logRetryCache)
@@ -7280,11 +7271,11 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         writeUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, poolNameStr, null, null);
+      logAuditEvent(false, operationName, poolNameStr);
       throw ace;
     }
-    logAuditEvent(true, operationName, poolNameStr, null, null);
     getEditLog().logSync();
+    logAuditEvent(true, operationName, poolNameStr);
   }
 
   BatchedListEntries<CachePoolEntry> listCachePools(String prevKey)
@@ -7302,10 +7293,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         readUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, null, null, null);
+      logAuditEvent(false, operationName, null);
       throw ace;
     }
-    logAuditEvent(true, operationName, null, null, null);
+    logAuditEvent(true, operationName, null);
     return results;
   }
 
@@ -7457,12 +7448,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       boolean logRetryCache) throws IOException, UnresolvedLinkException,
           SafeModeException, AccessControlException {
     final String operationName = "createEncryptionZone";
+    final FileStatus resultingStat;
     try {
       Metadata metadata = FSDirEncryptionZoneOp.ensureKeyIsInitialized(dir,
           keyName, src);
       final FSPermissionChecker pc = getPermissionChecker();
       checkOperation(OperationCategory.WRITE);
-      final FileStatus resultingStat;
       writeLock();
       try {
         checkSuperuserPrivilege(pc);
@@ -7473,13 +7464,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       } finally {
         writeUnlock(operationName);
       }
-
-      getEditLog().logSync();
-      logAuditEvent(true, operationName, src, null, resultingStat);
     } catch (AccessControlException e) {
       logAuditEvent(false, operationName, src);
       throw e;
     }
+    getEditLog().logSync();
+    logAuditEvent(true, operationName, src, null, resultingStat);
   }
 
   /**
@@ -7646,21 +7636,20 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     checkErasureCodingSupported(operationName);
     FileStatus resultingStat = null;
     final FSPermissionChecker pc = getPermissionChecker();
-    boolean success = false;
     writeLock();
     try {
       checkOperation(OperationCategory.WRITE);
       checkNameNodeSafeMode("Cannot set erasure coding policy on " + srcArg);
       resultingStat = FSDirErasureCodingOp.setErasureCodingPolicy(this,
           srcArg, ecPolicyName, pc, logRetryCache);
-      success = true;
+    } catch (AccessControlException ace) {
+      logAuditEvent(false, operationName, srcArg);
+      throw ace;
     } finally {
       writeUnlock(operationName);
-      if (success) {
-        getEditLog().logSync();
-      }
-      logAuditEvent(success, operationName, srcArg, null, resultingStat);
     }
+    getEditLog().logSync();
+    logAuditEvent(true, operationName, srcArg, null, resultingStat);
   }
 
   /**
@@ -7679,7 +7668,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     checkErasureCodingSupported(operationName);
     List<AddErasureCodingPolicyResponse> responses =
         new ArrayList<>(policies.length);
-    boolean success = false;
     writeLock();
     try {
       checkOperation(OperationCategory.WRITE);
@@ -7695,16 +7683,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
           responses.add(new AddErasureCodingPolicyResponse(policy, e));
         }
       }
-      success = true;
-      return responses.toArray(new AddErasureCodingPolicyResponse[0]);
     } finally {
       writeUnlock(operationName);
-      if (success) {
-        getEditLog().logSync();
-      }
-      logAuditEvent(success, operationName, addECPolicyNames.toString(),
-          null, null);
     }
+    getEditLog().logSync();
+    logAuditEvent(true, operationName, addECPolicyNames.toString());
+    return responses.toArray(new AddErasureCodingPolicyResponse[0]);
   }
 
   /**
@@ -7719,7 +7703,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     final String operationName = "removeErasureCodingPolicy";
     checkOperation(OperationCategory.WRITE);
     checkErasureCodingSupported(operationName);
-    boolean success = false;
     writeLock();
     try {
       checkOperation(OperationCategory.WRITE);
@@ -7727,14 +7710,11 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
           + ecPolicyName);
       FSDirErasureCodingOp.removeErasureCodingPolicy(this, ecPolicyName,
           logRetryCache);
-      success = true;
     } finally {
       writeUnlock(operationName);
-      if (success) {
-        getEditLog().logSync();
-      }
-      logAuditEvent(success, operationName, ecPolicyName, null, null);
     }
+    getEditLog().logSync();
+    logAuditEvent(true, operationName, ecPolicyName, null, null);
   }
 
   /**
@@ -7763,12 +7743,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         writeUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, ecPolicyName, null, null);
-    } finally {
-      if (success) {
-        getEditLog().logSync();
-        logAuditEvent(success, operationName, ecPolicyName, null, null);
-      }
+      logAuditEvent(false, operationName, ecPolicyName);
+      throw ace;
+    }
+    if (success) {
+      getEditLog().logSync();
+      logAuditEvent(true, operationName, ecPolicyName);
     }
     return success;
   }
@@ -7799,12 +7779,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
         writeUnlock(operationName);
       }
     } catch (AccessControlException ace) {
-      logAuditEvent(false, operationName, ecPolicyName, null, null);
-    } finally {
-      if (success) {
-        getEditLog().logSync();
-        logAuditEvent(success, operationName, ecPolicyName, null, null);
-      }
+      logAuditEvent(false, operationName, ecPolicyName);
+      throw ace;
+    }
+    if (success) {
+      getEditLog().logSync();
+      logAuditEvent(true, operationName, ecPolicyName);
     }
     return success;
   }
@@ -7824,21 +7804,17 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     checkErasureCodingSupported(operationName);
     FileStatus resultingStat = null;
     final FSPermissionChecker pc = getPermissionChecker();
-    boolean success = false;
     writeLock();
     try {
       checkOperation(OperationCategory.WRITE);
       checkNameNodeSafeMode("Cannot unset erasure coding policy on " + srcArg);
       resultingStat = FSDirErasureCodingOp.unsetErasureCodingPolicy(this,
           srcArg, pc, logRetryCache);
-      success = true;
     } finally {
       writeUnlock(operationName);
-      if (success) {
-        getEditLog().logSync();
-      }
-      logAuditEvent(success, operationName, srcArg, null, resultingStat);
     }
+    getEditLog().logSync();
+    logAuditEvent(true, operationName, srcArg, null, resultingStat);
   }
 
   /**
@@ -8045,6 +8021,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       logAuditEvent(false, operationName, src);
       throw e;
     }
+    logAuditEvent(true, operationName, src);
   }
 
   /**