[GitHub] [accumulo] milleruntime commented on a change in pull request #1614: Create max tablets property in new bulk import

[GitBox <gi...@apache.org>]

milleruntime commented on a change in pull request #1614:
URL: https://github.com/apache/accumulo/pull/1614#discussion_r433364473



##########
File path: test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java
##########
@@ -183,6 +186,29 @@ public void testSingleTabletSingleFileOffline() throws Exception {
     }
   }
 
+  @Test
+  public void testMaxTablets() throws Exception {
+    try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) {
+      tableName = "testMaxTablets_table1";
+      NewTableConfiguration newTableConf = new NewTableConfiguration();
+      // set logical time type so we can set time on bulk import
+      Map<String,String> props = new HashMap<>();
+      props.put(Property.TABLE_BULK_MAX_TABLETS.getKey(), "1");
+      newTableConf.setProperties(props);
+      client.tableOperations().create(tableName, newTableConf);
+
+      // test max tablets hit while inspecting bulk files
+      var thrown = assertThrows(RuntimeException.class, () -> testBulkFile(false, false));
+      var c = thrown.getCause();
+      assertTrue("Wrong exception: " + c, c instanceof ExecutionException);
+      assertTrue("Wrong exception: " + c.getCause(),
+          c.getCause() instanceof IllegalArgumentException);
+

Review comment:
       I have been looking at BulkNewIT to see how we could test this scenario and I think there is an issue with the test data.  This line doesn't make sense to me and seems wrong: https://github.com/milleruntime/accumulo/blob/f71a2d1ba175dbb74a8f9e26151b778c270eaa2c/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java#L315
   
   This also made me wonder about exclusivity when creating the load plan.  We don't mention whether the start and end rows are inclusive/exclusive on the load API [here](https://github.com/apache/accumulo/blob/3fd5cad92f9b63ac19e4466f3f2d5237b905262c/core/src/main/java/org/apache/accumulo/core/data/LoadPlan.java#L187).  I was wondering this myself and eventually found it mentioned in the ```RangeType``` enum but think it should also be mentioned on the ```loadFileTo``` methods, with the ```startRow``` and ```endRow``` parameters.




----------------------------------------------------------------
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.

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