[HDFS-14104] Review getImageTxIdToRetain

• Type: Improvement
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 3.0.0
• Fix Version/s: 3.3.0
• Component/s: namenode
• Hadoop Flags: Reviewed
• Flags: Patch
• Assignee: David Mollitor
• Reporter: David Mollitor
• Created: 27/Nov/18 17:43
• Updated: 30/Aug/19 00:14
• Resolved: 29/Aug/19 21:19

Description

NNStorageRetentionManager.java

  private long getImageTxIdToRetain(FSImageTransactionalStorageInspector inspector) {
      
    List<FSImageFile> images = inspector.getFoundImages();
    TreeSet<Long> imageTxIds = Sets.newTreeSet();
    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());    
    long minTxId = imageTxIdsList.get(toRetain - 1);
    LOG.info("Going to retain " + toRetain + " images with txid >= " +
        minTxId);
    return minTxId;
  }

1. Fix check style issues
2. Use SLF4J paramaterized logging
3. A lot of work gets done before checking if the list actually contains any entries and returning a 0. That should be the first thing that happens
4. Instead of building up the TreeSet in its natural order, then reversing the collection, simply use a reverse natural ordering to begin with and save a step.