keith-turner commented on code in PR #3957:
URL: https://github.com/apache/accumulo/pull/3957#discussion_r1398235422
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -1236,24 +1290,42 @@ private boolean isTabletAssigned(Key key) {
|| key.getColumnFamily().equals(FutureLocationColumnFamily.NAME);
}
- private KeyExtent getHighTablet(KeyExtent range) throws AccumuloException {
+ private HighTablet getHighTablet(KeyExtent range) throws AccumuloException {
try {
AccumuloClient client = manager.getContext();
Scanner scanner = client.createScanner(range.isMeta() ? RootTable.NAME :
MetadataTable.NAME,
Authorizations.EMPTY);
TabletColumnFamily.PREV_ROW_COLUMN.fetch(scanner);
+ MergedColumnFamily.MERGED_COLUMN.fetch(scanner);
KeyExtent start = new KeyExtent(range.tableId(), range.endRow(), null);
scanner.setRange(new Range(start.toMetaRow(), null));
Iterator<Entry<Key,Value>> iterator = scanner.iterator();
if (!iterator.hasNext()) {
throw new AccumuloException("No last tablet for a merge " + range);
}
- Entry<Key,Value> entry = iterator.next();
- KeyExtent highTablet = KeyExtent.fromMetaPrevRow(entry);
- if (!highTablet.tableId().equals(range.tableId())) {
+
+ KeyExtent highTablet = null;
+ boolean isMerged = false;
+ Value hasPrevRowColumn = null;
+
+ while (iterator.hasNext()) {
Review Comment:
I think an assumption of this loop is that its iterating over keys from the
same row. It would be good to validate that. In the case where the metadata
table is corrupt (like a tablet is missing a prev end row, it would be good to
fail here.
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -954,24 +999,32 @@ private void mergeMetadataRecords(MergeInfo info) throws
AccumuloException {
// delete any entries for external compactions
extCompIds.forEach(ecid ->
m.putDelete(ExternalCompactionColumnFamily.STR_NAME, ecid));
- if (!m.getUpdates().isEmpty()) {
- bw.addMutation(m);
- }
-
- bw.flush();
+ // Add a marker so we know the tablets have been fenced in case the
merge operation
+ // needs to be recovered and restarted to finish later. The value is
firstPrevRowValue
+ // which we need if we need to restart the operation
+ MergedColumnFamily.MERGED_COLUMN.put(m,
+ MergedColumnFamily.encodeHasPrevRowColumn(firstPrevRowValue !=
null));
Review Comment:
Can firstPrevRowValue ever be non null at this point? Bit uncertain about
this, thinking about the following when I ask this question.
If when merge operation starts there is only a single tablet in the merge
range, then it will never call `mergeMetadataRecords()`. So the first time
that `mergeMetadataRecords()` is called it should always see at least two
tablets and I think `firstPrevRowValue` is always non null when there are at
least two tablets. The first update made by `mergeMetadataRecords()` is done
before any tablets are deleted and adds the new merged marker. If the above is
true, then it seems when the merge marker is successfully added that it will
have seen at least two tablets and `firstPrevRowValue` will be non null?
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -1353,4 +1425,28 @@ private static void
markDeadServerLogsAsClosed(WalStateManager mgr,
}
}
+ @VisibleForTesting
+ protected static class HighTablet {
+ private final KeyExtent extent;
+ private final boolean merged;
+ private final boolean prevRowColumn;
+
+ public HighTablet(KeyExtent extent, boolean merged, boolean prevRowColumn)
{
+ this.extent = Objects.requireNonNull(extent);
+ this.prevRowColumn = prevRowColumn;
+ this.merged = merged;
+ }
+
+ public boolean isMerged() {
+ return merged;
+ }
+
+ public KeyExtent getExtent() {
+ return extent;
+ }
+
+ public boolean hasPrevRowColumn() {
Review Comment:
This name is confusing, it seems to be related to either one of the
following :
* There was a single tablet in the merge range when the merge marker was set
* Delete tablets still needs to be done
--
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]
