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

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


The following commit(s) were added to refs/heads/master by this push:
     new db9c2b9  Added unit test for too many tablets in bulk import (#1623)
db9c2b9 is described below

commit db9c2b975f2db1b3903402dea69e32a014d122da
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Mon Jun 15 11:43:56 2020 -0400

    Added unit test for too many tablets in bulk import (#1623)
---
 .../master/tableOps/bulkVer2/PrepBulkImport.java   | 14 ++--
 .../tableOps/bulkVer2/PrepBulkImportTest.java      | 92 +++++++++++++++++++---
 2 files changed, 89 insertions(+), 17 deletions(-)

diff --git 
a/server/master/src/main/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImport.java
 
b/server/master/src/main/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImport.java
index 7bc55af..058ad8e 100644
--- 
a/server/master/src/main/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImport.java
+++ 
b/server/master/src/main/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImport.java
@@ -27,6 +27,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 
@@ -43,7 +44,6 @@ import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.dataImpl.KeyExtent;
 import org.apache.accumulo.core.metadata.schema.TabletMetadata;
 import org.apache.accumulo.core.metadata.schema.TabletsMetadata;
-import org.apache.accumulo.fate.FateTxId;
 import org.apache.accumulo.fate.Repo;
 import org.apache.accumulo.master.Master;
 import org.apache.accumulo.master.tableOps.MasterRepo;
@@ -108,8 +108,12 @@ public class PrepBulkImport extends MasterRepo {
     return Objects.equals(extractor.apply(ke1), extractor.apply(ke2));
   }
 
+  /**
+   * Checks a load mapping to ensure all of the rows in the mapping exists in 
the table and that no
+   * file goes to too many tablets.
+   */
   @VisibleForTesting
-  static void checkForMerge(String tableId, LoadMappingIterator lmi,
+  static void sanityCheckLoadMapping(String tableId, LoadMappingIterator lmi,
       TabletIterFactory tabletIterFactory, int maxNumTablets, long tid) throws 
Exception {
     var currRange = lmi.next();
 
@@ -170,11 +174,9 @@ public class PrepBulkImport extends MasterRepo {
     if (maxNumTablets > 0) {
       fileCounts.values().removeIf(c -> c <= maxNumTablets);
       if (!fileCounts.isEmpty()) {
-        log.warn("{} Bulk files overlapped too many tablets : {}", 
FateTxId.formatTid(tid),
-            fileCounts);
         throw new AcceptableThriftTableOperationException(tableId, null, 
TableOperation.BULK_IMPORT,
             TableOperationExceptionType.OTHER, "Files overlap the configured 
max (" + maxNumTablets
-                + ") number of tablets: " + fileCounts.keySet());
+                + ") number of tablets: " + new TreeMap<>(fileCounts));
       }
     }
   }
@@ -194,7 +196,7 @@ public class PrepBulkImport extends MasterRepo {
           .forTable(bulkInfo.tableId).overlapping(startRow, 
null).checkConsistency().fetch(PREV_ROW)
           
.build(master.getContext()).stream().map(TabletMetadata::getExtent).iterator();
 
-      checkForMerge(bulkInfo.tableId.canonical(), lmi, tabletIterFactory, 
maxTablets, tid);
+      sanityCheckLoadMapping(bulkInfo.tableId.canonical(), lmi, 
tabletIterFactory, maxTablets, tid);
     }
   }
 
diff --git 
a/server/master/src/test/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImportTest.java
 
b/server/master/src/test/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImportTest.java
index 4d56526..de9a404 100644
--- 
a/server/master/src/test/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImportTest.java
+++ 
b/server/master/src/test/java/org/apache/accumulo/master/tableOps/bulkVer2/PrepBulkImportTest.java
@@ -18,6 +18,7 @@
  */
 package org.apache.accumulo.master.tableOps.bulkVer2;
 
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -28,8 +29,10 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
@@ -40,6 +43,7 @@ import 
org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationExcepti
 import org.apache.accumulo.core.clientImpl.bulk.Bulk;
 import org.apache.accumulo.core.clientImpl.bulk.BulkSerialize;
 import org.apache.accumulo.core.clientImpl.bulk.LoadMappingIterator;
+import 
org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
 import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.dataImpl.KeyExtent;
 import 
org.apache.accumulo.master.tableOps.bulkVer2.PrepBulkImport.TabletIterFactory;
@@ -87,7 +91,14 @@ public class PrepBulkImportTest {
     });
   }
 
-  public void runTest(List<KeyExtent> loadRanges, List<KeyExtent> 
tabletRanges) throws Exception {
+  private void runTest(List<KeyExtent> loadRanges, List<KeyExtent> 
tabletRanges) throws Exception {
+    Map<KeyExtent,String> lrm = new HashMap<>();
+    loadRanges.forEach(e -> lrm.put(e, "f1 f2 f3"));
+    runTest(lrm, tabletRanges, 100);
+  }
+
+  public void runTest(Map<KeyExtent,String> loadRanges, List<KeyExtent> 
tabletRanges,
+      int maxTablets) throws Exception {
     TabletIterFactory tabletIterFactory = startRow -> {
       int start = -1;
 
@@ -106,21 +117,25 @@ public class PrepBulkImportTest {
     };
 
     try (LoadMappingIterator lmi = createLoadMappingIter(loadRanges)) {
-      PrepBulkImport.checkForMerge("1", lmi, tabletIterFactory, 100, 10001);
+      PrepBulkImport.sanityCheckLoadMapping("1", lmi, tabletIterFactory, 
maxTablets, 10001);
     }
   }
 
-  private LoadMappingIterator createLoadMappingIter(List<KeyExtent> 
loadRanges) throws IOException {
+  private LoadMappingIterator createLoadMappingIter(Map<KeyExtent,String> 
loadRanges)
+      throws IOException {
     SortedMap<KeyExtent,Bulk.Files> mapping = new TreeMap<>();
-    Bulk.Files testFiles = new Bulk.Files();
 
-    long c = 0L;
-    for (String f : "f1 f2 f3".split(" ")) {
-      c++;
-      testFiles.add(new Bulk.FileInfo(f, c, c));
-    }
-    for (KeyExtent ke : loadRanges)
-      mapping.put(ke, testFiles);
+    loadRanges.forEach((extent, files) -> {
+      Bulk.Files testFiles = new Bulk.Files();
+      long c = 0L;
+      for (String f : files.split(" ")) {
+        c++;
+        testFiles.add(new Bulk.FileInfo(f, c, c));
+      }
+
+      mapping.put(extent, testFiles);
+    });
+
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
     BulkSerialize.writeLoadMapping(mapping, "/some/dir", p -> baos);
     ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
@@ -212,4 +227,59 @@ public class PrepBulkImportTest {
     }
 
   }
+
+  @Test
+  public void testTooManyTablets() throws Exception {
+    Map<KeyExtent,String> loadRanges = new HashMap<>();
+
+    loadRanges.put(nke(null, "c"), "f1 f2");
+    loadRanges.put(nke("c", "g"), "f2 f3");
+    loadRanges.put(nke("g", "r"), "f2 f4");
+    loadRanges.put(nke("r", "w"), "f2 f5");
+    loadRanges.put(nke("w", null), "f2 f6");
+
+    List<String> requiredRows = Arrays.asList("c", "g", "r", "w");
+    for (Set<String> otherRows : Sets.powerSet(Set.of("a", "e", "i", "s", 
"x"))) {
+
+      var tablets = Iterables.concat(requiredRows, otherRows);
+
+      for (int maxTablets = 3; maxTablets < 10; maxTablets++) {
+
+        int totalTablets = requiredRows.size() + otherRows.size() + 1;
+
+        if (totalTablets > maxTablets) {
+          runTooManyTest(loadRanges, tablets, "{f2=" + totalTablets + "}", 
maxTablets);
+        } else {
+          runTest(loadRanges, createExtents(tablets), maxTablets);
+        }
+      }
+
+      runTest(loadRanges, createExtents(tablets), 0);
+    }
+
+    loadRanges.clear();
+
+    loadRanges.put(nke("ca", "cz"), "f3");
+    loadRanges.put(nke("ma", "mm"), "f3");
+    loadRanges.put(nke("re", "rz"), "f4");
+
+    runTooManyTest(loadRanges, Arrays.asList("ca", "cd", "cz", "e", "ma", 
"md", "mm", "re", "rz"),
+        "{f3=4}", 3);
+    runTooManyTest(loadRanges,
+        Arrays.asList("b", "ca", "cd", "cz", "e", "ma", "md", "mm", "re", 
"rz"), "{f3=4}", 3);
+    runTooManyTest(loadRanges,
+        Arrays.asList("ca", "cd", "cz", "e", "ma", "md", "mm", "re", "rf", 
"rh", "rm", "rz"),
+        "{f3=4, f4=4}", 3);
+    runTooManyTest(loadRanges,
+        Arrays.asList("ca", "cd", "cz", "e", "ma", "mm", "re", "rf", "rh", 
"rm", "rz"), "{f4=4}",
+        3);
+  }
+
+  private void runTooManyTest(Map<KeyExtent,String> loadRanges, 
Iterable<String> tablets,
+      String expectedMessage, int maxTablets) {
+    var exception = assertThrows(ThriftTableOperationException.class,
+        () -> runTest(loadRanges, createExtents(tablets), maxTablets));
+    String message = exception.toString();
+    assertTrue(expectedMessage + " -- " + message, 
exception.toString().contains(expectedMessage));
+  }
 }

Reply via email to