From a54c053f830a41826767d018591e00ec7e35650d Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Tue, 31 Oct 2023 19:43:03 -0400 Subject: [PATCH] WIP adds support for splitting tablets with walogs Updates the FATE operations that split tablets to handle walogs. These changes should wait for a tablet with walogs to recover before proceeding. The changes to actually make recovery happen were already done in #3904 with changes to TabletGoalState.compute(). This change is a WIP because it depends on #3847 and needs those changes to be complete. fixes #3844 --- .../accumulo/core/metadata/schema/Ample.java | 2 ++ .../ConditionalTabletMutatorImpl.java | 12 +++++++++ .../manager/tableOps/merge/MergeTablets.java | 6 ++--- .../tableOps/split/DeleteOperationIds.java | 2 +- .../manager/tableOps/split/PreSplit.java | 25 ++++++++++++------- .../manager/tableOps/split/UpdateTablets.java | 7 ++++-- 6 files changed, 39 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java index a23e6da08e8..59d0d1ae849 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java @@ -501,6 +501,8 @@ interface ConditionalTabletMutator extends TabletUpdates * Ample provides the following features on top of the conditional writer to help automate diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java index a77a357d1f0..7fe8cf9f2e4 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java @@ -231,6 +231,10 @@ private void requireSameSingle(TabletMetadata tabletMetadata, ColumnType type) { mutation.addCondition(c); } break; + case LOGS: { + // TODO implement + } + break; default: throw new UnsupportedOperationException("Column type " + type + " is not supported."); } @@ -247,6 +251,14 @@ public Ample.ConditionalTabletMutator requireSame(TabletMetadata tabletMetadata, return this; } + @Override + public Ample.ConditionalTabletMutator requireAbsentLogs() { + Preconditions.checkState(updatesEnabled, "Cannot make updates after calling mutate."); + // create a tablet metadata with an empty set of logs and require the same as that + requireSameSingle(TabletMetadata.builder(extent).build(ColumnType.LOGS), ColumnType.LOGS); + return this; + } + @Override public void submit(Ample.RejectionHandler rejectionCheck) { Preconditions.checkState(updatesEnabled, "Cannot make updates after calling mutate."); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/merge/MergeTablets.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/merge/MergeTablets.java index ddb1616f806..a6fdf4081a8 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/merge/MergeTablets.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/merge/MergeTablets.java @@ -157,8 +157,8 @@ private void mergeMetadataRecords(Manager manager, long tid) throws AccumuloExce // update the last tablet try (var tabletsMutator = manager.getContext().getAmple().conditionallyMutateTablets()) { var lastExtent = lastTabletMeta.getExtent(); - var tabletMutator = - tabletsMutator.mutateTablet(lastExtent).requireOperation(opid).requireAbsentLocation(); + var tabletMutator = tabletsMutator.mutateTablet(lastExtent).requireOperation(opid) + .requireAbsentLocation().requireAbsentLogs(); // fence the files in the last tablet if needed lastTabletMeta.getFilesMap().forEach((file, dfv) -> { @@ -206,7 +206,7 @@ private void mergeMetadataRecords(Manager manager, long tid) throws AccumuloExce } var tabletMutator = tabletsMutator.mutateTablet(tabletMeta.getExtent()) - .requireOperation(opid).requireAbsentLocation(); + .requireOperation(opid).requireAbsentLocation().requireAbsentLogs(); tabletMeta.getKeyValues().keySet().forEach(key -> { log.debug("{} deleting {}", fateStr, key); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/DeleteOperationIds.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/DeleteOperationIds.java index a2c7169ea5a..d826b91b227 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/DeleteOperationIds.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/DeleteOperationIds.java @@ -53,7 +53,7 @@ public Repo call(long tid, Manager manager) throws Exception { splitInfo.getTablets().forEach(extent -> { tabletsMutator.mutateTablet(extent).requireOperation(opid).requireAbsentLocation() - .deleteOperation().submit(rejectionHandler); + .requireAbsentLogs().deleteOperation().submit(rejectionHandler); }); var results = tabletsMutator.process(); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/PreSplit.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/PreSplit.java index 6704ef6d545..c16b241decf 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/PreSplit.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/PreSplit.java @@ -19,6 +19,7 @@ package org.apache.accumulo.manager.tableOps.split; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOCATION; +import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOGS; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.OPID; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.PREV_ROW; @@ -77,7 +78,7 @@ public long isReady(long tid, Manager manager) throws Exception { // through as quickly as possible. var tabletMetadata = manager.getContext().getAmple().readTablet(splitInfo.getOriginal(), - PREV_ROW, LOCATION, OPID); + PREV_ROW, LOCATION, OPID, LOGS); log.trace("Attempting tablet split {} {} {}", FateTxId.formatTid(tid), splitInfo.getOriginal(), tabletMetadata == null ? null : tabletMetadata.getLocation()); @@ -87,28 +88,30 @@ public long isReady(long tid, Manager manager) throws Exception { // tablet no longer exists or is reserved by another operation return 0; } else if (opid.equals(tabletMetadata.getOperationId())) { - if (tabletMetadata.getLocation() == null) { - // the operation id is set and there is no location, so can proceed to split + if (tabletMetadata.getLocation() == null && tabletMetadata.getLogs().isEmpty()) { + // the operation id is set and there is no location or wals, so can proceed to split return 0; } else { - // the operation id was set, but a location is also set wait for it be unset + // the operation id was set, but a location or wals are also set, so wait for them to be + // unset return 1000; } } else { try (var tabletsMutator = manager.getContext().getAmple().conditionallyMutateTablets()) { tabletsMutator.mutateTablet(splitInfo.getOriginal()).requireAbsentOperation() - .requireSame(tabletMetadata, LOCATION).putOperation(opid) + .requireSame(tabletMetadata, LOCATION, LOGS).putOperation(opid) .submit(tmeta -> opid.equals(tmeta.getOperationId())); Map results = tabletsMutator.process(); if (results.get(splitInfo.getOriginal()).getStatus() == Status.ACCEPTED) { log.trace("Successfully set operation id for split {}", FateTxId.formatTid(tid)); - if (tabletMetadata.getLocation() == null) { - // the operation id was set and there is no location, so can move on + if (tabletMetadata.getLocation() == null && tabletMetadata.getLogs().isEmpty()) { + // the operation id was set and there is no location or wals, so can move on return 0; } else { - // now that the operation id set, generate an event to unload the tablet + // now that the operation id set, generate an event to unload the tablet or recover the + // logs manager.getEventCoordinator().event(splitInfo.getOriginal(), "Set operation id %s on tablet for split", FateTxId.formatTid(tid)); // the operation id was set, but a location is also set wait for it be unset @@ -129,7 +132,7 @@ public Repo call(long tid, Manager manager) throws Exception { manager.getSplitter().removeSplitStarting(splitInfo.getOriginal()); TabletMetadata tabletMetadata = manager.getContext().getAmple() - .readTablet(splitInfo.getOriginal(), PREV_ROW, LOCATION, OPID); + .readTablet(splitInfo.getOriginal(), PREV_ROW, LOCATION, OPID, LOGS); var opid = TabletOperationId.from(TabletOperationType.SPLITTING, tid); @@ -150,6 +153,10 @@ public Repo call(long tid, Manager manager) throws Exception { "Tablet unexpectedly had location set %s %s %s", FateTxId.formatTid(tid), tabletMetadata.getLocation(), tabletMetadata.getExtent()); + Preconditions.checkState(tabletMetadata.getLogs().isEmpty(), + "Tablet unexpectedly had walogs %s %s %s", FateTxId.formatTid(tid), + tabletMetadata.getLogs(), tabletMetadata.getExtent()); + // Create the dir name here for the next step. If the next step fails it will always have the // same dir name each time it runs again making it idempotent. diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/UpdateTablets.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/UpdateTablets.java index 246c73d8cee..08ef6003cb4 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/UpdateTablets.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/UpdateTablets.java @@ -90,6 +90,10 @@ public Repo call(long tid, Manager manager) throws Exception { "Tablet %s unexpectedly has a location %s", splitInfo.getOriginal(), tabletMetadata.getLocation()); + Preconditions.checkState(tabletMetadata.getLogs().isEmpty(), + "Tablet unexpectedly had walogs %s %s %s", FateTxId.formatTid(tid), + tabletMetadata.getLogs(), tabletMetadata.getExtent()); + var newTablets = splitInfo.getTablets(); var newTabletsFiles = getNewTabletFiles(newTablets, tabletMetadata, @@ -99,7 +103,6 @@ public Repo call(long tid, Manager manager) throws Exception { // Only update the original tablet after successfully creating the new tablets, this is // important for failure cases where this operation partially runs a then runs again. - updateExistingTablet(tid, manager, tabletMetadata, opid, newTablets, newTabletsFiles); return new DeleteOperationIds(splitInfo); @@ -217,7 +220,7 @@ private void updateExistingTablet(long tid, Manager manager, TabletMetadata tabl var newExtent = newTablets.last(); var mutator = tabletsMutator.mutateTablet(splitInfo.getOriginal()).requireOperation(opid) - .requireAbsentLocation(); + .requireAbsentLocation().requireAbsentLogs(); mutator.putPrevEndRow(newExtent.prevEndRow());