Address review comments

Author: NihalJain

hbase-client/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotTTLExpiredException.java

@@ -28,14 +28,16 @@
 @InterfaceAudience.Public
 public class SnapshotTTLExpiredException extends HBaseSnapshotException {
   /**
+   * Failure when the ttl for snapshot has already expired.
    * @param message the full description of the failure
    */
   public SnapshotTTLExpiredException(String message) {
     super(message);
   }
 
   /**
-   * @param snapshotDescription expected snapshot to find
+   * Failure when the ttl for snapshot has already expired.
+   * @param snapshotDescription snapshot that was attempted
    */
   public SnapshotTTLExpiredException(SnapshotDescription snapshotDescription) {
     super("TTL for snapshot '" + snapshotDescription.getName() + "' has already expired.",

hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java

@@ -470,12 +470,12 @@ public ListMultimap<String, UserPermission> run() throws Exception {
    * Method to check whether TTL has expired for specified snapshot creation time and snapshot ttl.
    * NOTE: For backward compatibility (after the patch deployment on HMaster), any snapshot with ttl
    * 0 is to be considered as snapshot to keep FOREVER. Default ttl value specified by
-   * {@HConstants.DEFAULT_SNAPSHOT_TTL}
+   * {@link HConstants.DEFAULT_SNAPSHOT_TTL}
    * @return true if ttl has expired, or, false, otherwise
    */
   public static boolean isExpiredSnapshot(long snapshotTtl, long snapshotCreatedTime,
     long currentTime) {
-    return snapshotCreatedTime > 0 && snapshotTtl > 0
+    return snapshotCreatedTime > 0 && snapshotTtl > HConstants.DEFAULT_SNAPSHOT_TTL
       && snapshotTtl < TimeUnit.MILLISECONDS.toSeconds(Long.MAX_VALUE)
       && (snapshotCreatedTime + TimeUnit.SECONDS.toMillis(snapshotTtl)) < currentTime;
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotWithTTLFromClient.java

@@ -18,6 +18,8 @@
 package org.apache.hadoop.hbase.client;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
@@ -115,7 +117,7 @@ public void testRestoreSnapshotWithTTLSuccess() throws Exception {
     String snapshotName = "nonExpiredTTLRestoreSnapshotTest";
 
     // table should exist
-    assertEquals(true, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
 
     // create snapshot fo given table with specified ttl
     createSnapshotWithTTL(TABLE_NAME, snapshotName, CHORE_INTERVAL_SECS * 2);
@@ -124,21 +126,21 @@ public void testRestoreSnapshotWithTTLSuccess() throws Exception {
     // Disable and drop table
     admin.disableTable(TABLE_NAME);
     admin.deleteTable(TABLE_NAME);
-    assertEquals(false, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertFalse(UTIL.getAdmin().tableExists(TABLE_NAME));
 
     // restore snapshot
     admin.restoreSnapshot(snapshotName);
 
     // table should be created
-    assertEquals(true, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
   }
 
   @Test
   public void testRestoreSnapshotFailsDueToTTLExpired() throws Exception {
     String snapshotName = "expiredTTLRestoreSnapshotTest";
 
     // table should exist
-    assertEquals(true, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
 
     // create snapshot fo given table with specified ttl
     createSnapshotWithTTL(TABLE_NAME, snapshotName, 1);
@@ -147,7 +149,7 @@ public void testRestoreSnapshotFailsDueToTTLExpired() throws Exception {
     // Disable and drop table
     admin.disableTable(TABLE_NAME);
     admin.deleteTable(TABLE_NAME);
-    assertEquals(false, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertFalse(UTIL.getAdmin().tableExists(TABLE_NAME));
 
     // Sleep so that TTL may expire
     Threads.sleep(2000);
@@ -161,15 +163,15 @@ public void testRestoreSnapshotFailsDueToTTLExpired() throws Exception {
     }
 
     // table should not be created
-    assertEquals(false, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertFalse(UTIL.getAdmin().tableExists(TABLE_NAME));
   }
 
   @Test
   public void testCloneSnapshotWithTTLSuccess() throws Exception {
     String snapshotName = "nonExpiredTTLCloneSnapshotTest";
 
     // table should exist
-    assertEquals(true, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
 
     // create snapshot fo given table with specified ttl
     createSnapshotWithTTL(TABLE_NAME, snapshotName, CHORE_INTERVAL_SECS * 2);
@@ -179,21 +181,21 @@ public void testCloneSnapshotWithTTLSuccess() throws Exception {
     admin.cloneSnapshot(snapshotName, CLONED_TABLE_NAME);
 
     // table should be created
-    assertEquals(true, UTIL.getAdmin().tableExists(CLONED_TABLE_NAME));
+    assertTrue(UTIL.getAdmin().tableExists(CLONED_TABLE_NAME));
   }
 
   @Test
   public void testCloneSnapshotFailsDueToTTLExpired() throws Exception {
     String snapshotName = "expiredTTLCloneSnapshotTest";
 
     // table should exist
-    assertEquals(true, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
 
     // create snapshot fo given table with specified ttl
     createSnapshotWithTTL(TABLE_NAME, snapshotName, 1);
     Admin admin = UTIL.getAdmin();
 
-    assertEquals(true, UTIL.getAdmin().tableExists(TABLE_NAME));
+    assertTrue(UTIL.getAdmin().tableExists(TABLE_NAME));
 
     // Sleep so that TTL may expire
     Threads.sleep(2000);
@@ -207,7 +209,7 @@ public void testCloneSnapshotFailsDueToTTLExpired() throws Exception {
     }
 
     // table should not be created
-    assertEquals(false, UTIL.getAdmin().tableExists(CLONED_TABLE_NAME));
+    assertFalse(UTIL.getAdmin().tableExists(CLONED_TABLE_NAME));
   }
 
   private void createSnapshotWithTTL(TableName tableName, final String snapshotName,