[GitHub] [accumulo] EdColeman commented on a diff in pull request #3703: add check to gc to validate metadata scan is complete

["EdColeman (via GitHub)" <gi...@apache.org>]

EdColeman commented on code in PR #3703:
URL: https://github.com/apache/accumulo/pull/3703#discussion_r1297478399


##########
server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java:
##########
@@ -644,4 +768,47 @@ public void bulkImportReplicationRecordsPreventDeletion() throws Exception {
     assertEquals(1, gce.deletes.size());
     assertEquals("hdfs://foo.com:6000/accumulo/tables/2/t-00002/A000002.rf", gce.deletes.get(0));
   }
+
+  /**
+   * Minimal test to show that dir and prevRow are required for valid scan
+   */
+  @Test
+  public void testDirAndPrevRow() throws Exception {
+
+    GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
+
+    TestGCE gce = new TestGCE();
+    gce.candidates.add("/1636/default_tablet");
+    gce.addDirReference("1636", null, "/default_tablet");
+    gce.addPrevRowReference("1636", null);
+    gca.collect(gce);
+  }
+
+  /**
+   * Show that IllegalState is thrown when no dir entry present in metadata scan.
+   */
+  @Test
+  public void testNoDir() {

Review Comment:
   Maybe there should be a test for the last tablet? But being this is using the TestGCE - does it really need two tablets.  The metadata check is not following the chain - just looking that an entry exists.  The check will pass if any entry is seen - even if it was valid (say point to a non-existing tablet)
   
   This seemed to be the minimum about of code to trigger the exception.  Is that not valid?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org