keith-turner commented on code in PR #3903:
URL: https://github.com/apache/accumulo/pull/3903#discussion_r1375263773
##########
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/merge/MergeTablets.java:
##########
@@ -233,6 +233,9 @@ static void validateTablet(TabletMetadata tabletMeta,
String fateStr, TabletOper
Preconditions.checkState(expectedTableId.equals(tabletMeta.getTableId()),
"%s tablet %s has unexpected table id %s expected %s", fateStr,
tabletMeta.getExtent(),
tabletMeta.getTableId(), expectedTableId);
+ Preconditions.checkState(tabletMeta.getLogs().isEmpty(),
Review Comment:
For the method this call this if they are fetching columns, then need to add
the WALS. Like DeleteRows calls this and is fetching columns.
##########
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/merge/ReserveTablets.java:
##########
@@ -77,15 +78,17 @@ public long isReady(long tid, Manager env) throws Exception
{
locations++;
}
+ wals += tabletMeta.getLogs().size();
Review Comment:
Need WALS to the coumns to fetch above or this will throw an exception
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -396,7 +396,13 @@ private TableMgmtStats
manageTablets(Iterator<TabletManagement> iter,
}
if (tm.getOperationId() != null) {
- goal = TabletGoalState.UNASSIGNED;
+ // If there are still wals the tablet needs to be hosted
+ // to process the wals before starting the op
+ if (tm.getLogs().isEmpty()) {
+ goal = TabletGoalState.UNASSIGNED;
+ } else {
Review Comment:
May want to limit this to hosting on merge operations with wals, so if opid
is merge and it has wals then host. Eventually split will need it too (#3844),
but may be good to make the FATE aware of logs. Delete operations probably
will never need tablets hosted for recovery.
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -279,6 +279,11 @@ private boolean shouldReturnDueToLocation(final
TabletMetadata tm,
|| (tm.getHostingGoal() == TabletHostingGoal.ONDEMAND &&
tm.getHostingRequested()))) {
return true;
}
+ // If the Tablet has walogs and operation id then need to return so
+ // TGW can bring online to process the logs
+ if (tm.getLogs().isEmpty() && tm.getOperationId() != null) {
Review Comment:
May want to limit this to merging operation for now. Split will need this
once its fully implement (#3844). Delete table will never need it.
```suggestion
if (tm.getLogs().isEmpty() && tm.getOperationId() != null &&
tm.getOperationId().getType() == TabletOperationType.MERGING) {
```
--
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]
