keith-turner commented on code in PR #4229:
URL: https://github.com/apache/accumulo/pull/4229#discussion_r1480065021
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -249,22 +249,29 @@ private void computeTabletManagementActions(final
TabletMetadata tm,
if (tm.isFutureAndCurrentLocationSet()) {
// no need to check everything, we are in a known state where we want to
return everything.
reasonsToReturnThisTablet.add(ManagementAction.BAD_STATE);
+ // return early, operations can't occur in bad state
+ return;
+ }
+
+ if (!tm.getLogs().isEmpty() && (tm.getOperationId() == null
+ || tm.getOperationId().getType() != TabletOperationType.DELETING)) {
+ reasonsToReturnThisTablet.add(ManagementAction.NEEDS_RECOVERY);
+ // return early, we need recovery to occur before normal tablet
operations (migration, split,
+ // compact, etc.)
return;
}
if
(VolumeUtil.needsVolumeReplacement(tabletMgmtParams.getVolumeReplacements(),
tm)) {
reasonsToReturnThisTablet.add(ManagementAction.NEEDS_VOLUME_REPLACEMENT);
+ // return early, we need volume replacement to occur before normal
tablet operations
+ // (migration, split, compact, etc.)
+ return;
Review Comment:
Not sure how this suggestion interacts with the tablet mgmt code. Given
that getting vol replacement done is important and it stops happening when
nothing returns, maybe the two new returns should be removed and this added
after the if stmts.
```java
if(!reasonsToReturnThisTablet.isEmpty()){
return;
}
```
Alternatively, may the check for vol replacement should be first (even
before the BAD_STATE check) and return. Thinking this because of the way the
tablet mgmt code handle vol replacement, it stops requesting checks for it when
none is seen. So maybe it should come first and have a comment saying it needs
to be first.
--
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]
