keith-turner commented on code in PR #3835:
URL: https://github.com/apache/accumulo/pull/3835#discussion_r1360715247
##########
test/src/main/java/org/apache/accumulo/test/functional/ManagerAssignmentIT.java:
##########
@@ -413,24 +415,41 @@ public void testOpidPreventsAssignment() throws Exception
{
writer.addMutation(m);
}
+ // Host all tablets.
c.tableOperations().setTabletHostingGoal(tableName, new Range(),
TabletHostingGoal.ALWAYS);
+ Wait.waitFor(() -> countTabletsWithLocation(c, tableId) == 3);
Review Comment:
Could make the test check the tablet after waiting to ensure the expected
tablet does not have a location
```suggestion
Wait.waitFor(() -> countTabletsWithLocation(c, tableId) == 3);
var ample = ((ClientContext)c).getAmple();
assertNull(ample.readTablet(new KeyExtent(tableId, new Text("m"), new
Text("f"))).getLocation());
```
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -312,7 +313,8 @@ private boolean shouldReturnDueToLocation(final
TabletMetadata tm,
return true;
case SUSPENDED:
case UNASSIGNED:
- if (shouldBeOnline && (tm.getHostingGoal() == TabletHostingGoal.ALWAYS
+ if (shouldBeOnline && tm.getOperationId() == null &&
(tm.getHostingGoal()
Review Comment:
Why check the operation id is null here?
##########
test/src/main/java/org/apache/accumulo/test/functional/ManagerAssignmentIT.java:
##########
@@ -413,24 +415,41 @@ public void testOpidPreventsAssignment() throws Exception
{
writer.addMutation(m);
}
+ // Host all tablets.
c.tableOperations().setTabletHostingGoal(tableName, new Range(),
TabletHostingGoal.ALWAYS);
+ Wait.waitFor(() -> countTabletsWithLocation(c, tableId) == 3);
- Wait.waitFor(() -> countTabletsWithLocation(c, tableId) >= 3);
+ // Delete the OperationId column, tablet should be assigned
+ try (var writer = c.createBatchWriter(MetadataTable.NAME)) {
+ var extent = new KeyExtent(tableId, new Text("m"), new Text("f"));
+ Mutation m = new Mutation(extent.toMetaRow());
+ TabletsSection.ServerColumnFamily.OPID_COLUMN.putDelete(m);
+ writer.addMutation(m);
+ }
+ Wait.waitFor(() -> countTabletsWithLocation(c, tableId) == 4);
- // there are four tablets, but one has an operation id set and should
not be assigned
- assertEquals(3, countTabletsWithLocation(c, tableId));
+ // Set the OperationId on one tablet, which will cause that tablet
+ // to be unhosted
+ try (var writer = c.createBatchWriter(MetadataTable.NAME)) {
+ var extent = new KeyExtent(tableId, new Text("m"), new Text("f"));
+ var opid = TabletOperationId.from(TabletOperationType.SPLITTING, 42L);
+ Mutation m = new Mutation(extent.toMetaRow());
+ TabletsSection.ServerColumnFamily.OPID_COLUMN.put(m, new
Value(opid.canonical()));
+ writer.addMutation(m);
+ }
+ // there are four tablets, three should be assigned as one has a
OperationId
+ Wait.waitFor(() -> countTabletsWithLocation(c, tableId) == 3);
Review Comment:
Could also add a check here
```suggestion
Wait.waitFor(() -> countTabletsWithLocation(c, tableId) == 3);
assertNull(ample.readTablet(new KeyExtent(tableId, new Text("m"), new
Text("f"))).getLocation());
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
