[accumulo] branch main updated: Reword some comments to clarify code (#2260)

[mm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mmiller pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
     new 647b03a  Reword some comments to clarify code (#2260)
647b03a is described below

commit 647b03a231c43d84e1a949668bb3c3cc8fa1b917
Author: Mike Miller <mm...@apache.org>
AuthorDate: Thu Sep 2 14:58:58 2021 -0400

    Reword some comments to clarify code (#2260)
    
    * Also rename some methods in CompactableImpl
    * Drop confusing sentence in comments
    * Small improvements to comments
    
    Co-authored-by: Dom G <47...@users.noreply.github.com>
---
 .../accumulo/tserver/tablet/CompactableImpl.java   | 32 ++++++++++------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java
index 47b1861..1e60d1c 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java
@@ -171,7 +171,7 @@ public class CompactableImpl implements Compactable {
    *
    * <p>
    * This class does no synchronization of its own and relies on CompactableImpl to do all needed
-   * synchronization. CompactableImpl must makes changes to files and other state like running jobs
+   * synchronization. CompactableImpl must make changes to files and other state like running jobs
    * in a mutually exclusive manner, so synchronization at this level is unnecessary.
    *
    */
@@ -181,10 +181,9 @@ public class CompactableImpl implements Compactable {
     private CompactionKind selectKind = null;
 
     // Tracks if when a set of files was selected, if at that time the set was all of the tablets
-    // files. Because a set of selected files can be compacted over one or more compactions, its
+    // files. Because a set of selected files can be compacted over one or more compactions, it's
     // important to track this in order to know if the last compaction is a full compaction and
-    // should
-    // not propagate deletes.
+    // should not propagate deletes.
     private boolean initiallySelectedAll = false;
     private Set<StoredTabletFile> selectedFiles = new HashSet<>();
 
@@ -362,7 +361,7 @@ public class CompactableImpl implements Compactable {
         log.trace("Ignoring because compacting not a subset {}", getExtent());
 
         // A compaction finished, so things are out of date. This can happen because CompactableImpl
-        // and Tablet have separate locks, its ok.
+        // and Tablet have separate locks, it's ok.
         return Set.of();
       }
 
@@ -561,9 +560,9 @@ public class CompactableImpl implements Compactable {
 
     Map<ExternalCompactionId,String> extCompactionsToRemove = new HashMap<>();
 
-    var extSelInfo = initializeSelection(extCompactions, tablet, extCompactionsToRemove);
+    var extSelInfo = initExternalSelection(extCompactions, tablet, extCompactionsToRemove);
 
-    sanityCheckExternalCompactions(extCompactions, dataFileSizes.keySet(), extCompactionsToRemove);
+    verifyExternalCompactions(extCompactions, dataFileSizes.keySet(), extCompactionsToRemove);
 
     extCompactionsToRemove.forEach((ecid, reason) -> {
       log.warn("Removing external compaction {} for {} because {} meta: {}", ecid,
@@ -615,7 +614,7 @@ public class CompactableImpl implements Compactable {
     this.fileMgr = new FileManager(tablet.getExtent(), extCompactingFiles, extSelInfo);
   }
 
-  private void sanityCheckExternalCompactions(
+  private void verifyExternalCompactions(
       Map<ExternalCompactionId,ExternalCompactionMetadata> extCompactions,
       Set<StoredTabletFile> tabletFiles, Map<ExternalCompactionId,String> extCompactionsToRemove) {
 
@@ -635,7 +634,6 @@ public class CompactableImpl implements Compactable {
         extCompactionsToRemove.putIfAbsent(ecid, "Some external compaction files overlap");
       });
     }
-
   }
 
   private synchronized boolean addJob(CompactionJob job) {
@@ -760,14 +758,14 @@ public class CompactableImpl implements Compactable {
   }
 
   /**
-   * For user compactions a set of files is selected. Then those files compacted by one or more
+   * For user compactions a set of files is selected. Those files then get compacted by one or more
    * compactions until the set is empty. This method attempts to reconstruct the selected set of
-   * files when a tablet is loaded that has an external user compaction. Doing this avoid repeating
-   * work and also when a user compaction completes it does checks against the selected set. So the
-   * selected set must be initialized. Since the data is coming from persisted storage, lots of
-   * sanity checks are done in this method rather than assuming the persisted data is just correct.
+   * files when a tablet is loaded with an external user compaction. It avoids repeating work and
+   * when a user compaction completes, files are verified against the selected set. Since the data
+   * is coming from persisted storage, lots of checks are done in this method rather than assuming
+   * the persisted data is correct.
    */
-  private Optional<SelectedInfo> initializeSelection(
+  private Optional<SelectedInfo> initExternalSelection(
       Map<ExternalCompactionId,ExternalCompactionMetadata> extCompactions, Tablet tablet,
       Map<ExternalCompactionId,String> externalCompactionsToRemove) {
     CompactionKind extKind = null;
@@ -1046,7 +1044,7 @@ public class CompactableImpl implements Compactable {
   }
 
   /**
-   * Attempt to reserve files for compaction. Its possible that since a compaction job was queued
+   * Attempt to reserve files for compaction. It's possible that since a compaction job was queued
    * that things have changed and there is no longer anything to do for the job. In this case
    * Optional.empty() is returned.
    */
@@ -1392,7 +1390,7 @@ public class CompactableImpl implements Compactable {
   }
 
   /**
-   * Interrupts and waits for any running compactions. After this method returns no compactions
+   * Interrupts and waits for any running compactions. After this method returns, no compactions
    * should be running and none should be able to start.
    */
   public synchronized void close() {