From ffca734c62fba26211f22232ddb5e80eae4b5d51 Mon Sep 17 00:00:00 2001
From: Inigo Goiri <inigoiri@apache.org>
Date: Thu, 29 Aug 2019 14:18:35 -0700
Subject: [PATCH] HDFS-14104. Review getImageTxIdToRetain. Contributed by David
 Mollitor.

---
 .../namenode/NNStorageRetentionManager.java   | 27 +++++++++----------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java
index fc54dfcfd31..d0fa00314d7 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java
@@ -184,27 +184,26 @@ public class NNStorageRetentionManager {
    * @return the transaction ID corresponding to the oldest checkpoint
    * that should be retained. 
    */
-  private long getImageTxIdToRetain(FSImageTransactionalStorageInspector inspector) {
-      
-    List<FSImageFile> images = inspector.getFoundImages();
-    TreeSet<Long> imageTxIds = Sets.newTreeSet();
+  private long getImageTxIdToRetain(
+      FSImageTransactionalStorageInspector inspector) {
+
+    final List<FSImageFile> images = inspector.getFoundImages();
+    if (images.isEmpty()) {
+      return 0L;
+    }
+
+    TreeSet<Long> imageTxIds = Sets.newTreeSet(Collections.reverseOrder());
     for (FSImageFile image : images) {
       imageTxIds.add(image.getCheckpointTxId());
     }
-    
+
     List<Long> imageTxIdsList = Lists.newArrayList(imageTxIds);
-    if (imageTxIdsList.isEmpty()) {
-      return 0;
-    }
-    
-    Collections.reverse(imageTxIdsList);
-    int toRetain = Math.min(numCheckpointsToRetain, imageTxIdsList.size());    
+    int toRetain = Math.min(numCheckpointsToRetain, imageTxIdsList.size());
     long minTxId = imageTxIdsList.get(toRetain - 1);
-    LOG.info("Going to retain " + toRetain + " images with txid >= " +
-        minTxId);
+    LOG.info("Going to retain {} images with txid >= {}", toRetain, minTxId);
     return minTxId;
   }
-  
+
   /**
    * Interface responsible for disposing of old checkpoints and edit logs.
    */