keith-turner commented on code in PR #3900:
URL: https://github.com/apache/accumulo/pull/3900#discussion_r1374886853
##########
server/manager/src/main/java/org/apache/accumulo/manager/state/MergeStats.java:
##########
@@ -271,6 +281,22 @@ private boolean verifyMergeConsistency(AccumuloClient
accumuloClient, CurrentSta
&& unassigned == verify.total;
}
+ @VisibleForTesting
+ boolean verifyState(MergeInfo info) {
+ if (info.getState() != MergeState.WAITING_FOR_OFFLINE) {
+ return false;
+ }
+ return true;
+ }
+
+ @VisibleForTesting
+ boolean verifyWalogs(TabletLocationState tls) {
+ if (!tls.walogs.isEmpty()) {
+ return false;
+ }
+ return true;
Review Comment:
```suggestion
return tls.walogs.isEmpty();
```
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -783,13 +784,24 @@ private void mergeMetadataRecords(MergeInfo info) throws
AccumuloException {
TabletColumnFamily.PREV_ROW_COLUMN.fetch(scanner);
ServerColumnFamily.TIME_COLUMN.fetch(scanner);
ServerColumnFamily.DIRECTORY_COLUMN.fetch(scanner);
+
scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
+ scanner.fetchColumnFamily(CurrentLocationColumnFamily.NAME);
+ scanner.fetchColumnFamily(LogColumnFamily.NAME);
Mutation m = new Mutation(stopRow);
MetadataTime maxLogicalTime = null;
for (Entry<Key,Value> entry : scanner) {
Key key = entry.getKey();
Value value = entry.getValue();
- if (key.getColumnFamily().equals(DataFileColumnFamily.NAME)) {
+
+ // Verify that Tablet is offline
+ if (key.getColumnFamily().equals(CurrentLocationColumnFamily.NAME)) {
Review Comment:
Can also check for a future location, should not see that either.
```suggestion
if (key.getColumnFamily().equals(CurrentLocationColumnFamily.NAME)
|| key.getColumnFamily().equals(FutureLocationColumnFamily.NAME)) {
```
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -783,13 +784,24 @@ private void mergeMetadataRecords(MergeInfo info) throws
AccumuloException {
TabletColumnFamily.PREV_ROW_COLUMN.fetch(scanner);
ServerColumnFamily.TIME_COLUMN.fetch(scanner);
ServerColumnFamily.DIRECTORY_COLUMN.fetch(scanner);
+
scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
+ scanner.fetchColumnFamily(CurrentLocationColumnFamily.NAME);
Review Comment:
```suggestion
scanner.fetchColumnFamily(CurrentLocationColumnFamily.NAME);
scanner.fetchColumnFamily(FutureLocationColumnFamily.NAME);
```
##########
server/manager/src/main/java/org/apache/accumulo/manager/state/MergeStats.java:
##########
@@ -271,6 +281,22 @@ private boolean verifyMergeConsistency(AccumuloClient
accumuloClient, CurrentSta
&& unassigned == verify.total;
}
+ @VisibleForTesting
+ boolean verifyState(MergeInfo info) {
+ if (info.getState() != MergeState.WAITING_FOR_OFFLINE) {
+ return false;
+ }
+ return true;
Review Comment:
Another merge state seems really unexpected here, wonder if an exception
would be better.
```suggestion
Preconditions.checkstate(info.getState() ==
MergeState.WAITING_FOR_OFFLINE, "Unexpected merge state %", info.getState());
if (info.getState() != MergeState.WAITING_FOR_OFFLINE) {
return false;
}
return true;
```
--
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]
