[GitHub] [accumulo] dlmarion commented on a diff in pull request #3350: modifies bulk import to use conditional mutations

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

dlmarion commented on code in PR #3350:
URL: https://github.com/apache/accumulo/pull/3350#discussion_r1186253911


##########
test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java:
##########
@@ -180,6 +181,8 @@ public void testSingleTabletSingleFile() throws Exception {
   }
 
   @Test
+  @Disabled("Need to implement set time functionality")

Review Comment:
   I think we should leave the test broken. I saw the follow-on issue that you created to fix this.



##########
test/src/main/java/org/apache/accumulo/test/functional/BulkSplitOptimizationIT.java:
##########
@@ -105,6 +107,8 @@ public void testBulkSplitOptimization() throws Exception {
       // initiate splits
       c.tableOperations().setProperty(tableName, Property.TABLE_SPLIT_THRESHOLD.getKey(), "100K");
 
+      c.tableOperations().setTabletHostingGoal(tableName, new Range(), TabletHostingGoal.ALWAYS);

Review Comment:
   Are you adding this so that the test passes? If so, should we leave the test broken so that we can fix it when we move the split logic to the Manager for hosted and unhosted tablets?



##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -119,6 +119,7 @@ public class TabletMetadata {
   private TabletHostingGoal goal = TabletHostingGoal.ONDEMAND;
   private boolean onDemandHostingRequested = false;
   private TabletOperationId operationId;
+  private Map<Long,TServerInstance> refreshIds;

Review Comment:
   is this still being used?



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