This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push:
new 1d7f6937fc Considers all tablet metadata columns in split code (#4323)
1d7f6937fc is described below
commit 1d7f6937fcd4a122978486c10832b37c62effea1
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Mon Mar 4 10:31:48 2024 -0500
Considers all tablet metadata columns in split code (#4323)
Made the following changes to the split code that adds new tablets and
updates the existing tablet.
* fixed potential NPE w/ tablet operation id check by reversing order
of equals check
* Throws IllegalStateException when attempting to split tablet with
merged or cloned markers
* Removed adding wals when creating new tablets in split, its not
expected that the parent tablet would have wals and this is checked
earlier
* Deleted any user compaction requested, hosting requested, suspended,
or last columns in the parent tablet
Added a unit test that attempts to exercise the split code with all
tablet columns. The unit test also has a set of tablet columns that
were verified to work with split and it is checked against the set of
columns in the code. The purpose of this test is to fail when a new
column is added to ensure that split is considered.
Was a bit uncertain about deleting the last location and suspend. Those
columns either need to be deleted from the parent tablet or added to the
new tablets being created. The current code was doing neither. Decided
to delete them as the new tablets have a different range and are
conceptually different tablets than the parent.
---
.../manager/tableOps/split/UpdateTablets.java | 42 ++-
.../manager/tableOps/split/UpdateTabletsTest.java | 328 +++++++++++++++++++++
.../apache/accumulo/test/functional/SplitIT.java | 54 ++++
3 files changed, 421 insertions(+), 3 deletions(-)
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 2fe3c56399..f2d1501ae5 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
@@ -22,6 +22,7 @@ import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeMap;
@@ -82,7 +83,7 @@ public class UpdateTablets extends ManagerRepo {
}
}
- Preconditions.checkState(tabletMetadata.getOperationId().equals(opid),
+ Preconditions.checkState(opid.equals(tabletMetadata.getOperationId()),
"Tablet %s does not have expected operation id %s it has %s",
splitInfo.getOriginal(), opid,
tabletMetadata.getOperationId());
@@ -94,6 +95,13 @@ public class UpdateTablets extends ManagerRepo {
"Tablet unexpectedly had walogs %s %s %s", fateId,
tabletMetadata.getLogs(),
tabletMetadata.getExtent());
+ Preconditions.checkState(!tabletMetadata.hasMerged(),
+ "Tablet unexpectedly has a merged marker %s %s", fateId,
tabletMetadata.getExtent());
+
+ Preconditions.checkState(tabletMetadata.getCloned() == null,
+ "Tablet unexpectedly has a cloned marker %s %s %s", fateId,
tabletMetadata.getCloned(),
+ tabletMetadata.getExtent());
+
var newTablets = splitInfo.getTablets();
var newTabletsFiles = getNewTabletFiles(newTablets, tabletMetadata,
@@ -120,7 +128,7 @@ public class UpdateTablets extends ManagerRepo {
newTablets.forEach(extent -> tabletsFiles.put(extent, new HashMap<>()));
- // determine while files overlap which tablets and their estimated sizes
+ // determine which files overlap which tablets and their estimated sizes
tabletMetadata.getFilesMap().forEach((file, dataFileValue) -> {
FileUtil.FileInfo fileInfo = fileInfoProvider.apply(file);
@@ -187,6 +195,7 @@ public class UpdateTablets extends ManagerRepo {
mutator.putTime(tabletMetadata.getTime());
tabletMetadata.getFlushId().ifPresent(mutator::putFlushId);
mutator.putPrevEndRow(newExtent.prevEndRow());
+
tabletMetadata.getCompacted().forEach(mutator::putCompacted);
tabletMetadata.getCompacted().forEach(compactedFateId -> log
@@ -195,7 +204,6 @@ public class UpdateTablets extends ManagerRepo {
mutator.putTabletAvailability(tabletMetadata.getTabletAvailability());
tabletMetadata.getLoaded().forEach((k, v) ->
mutator.putBulkFile(k.getTabletFile(), v));
- tabletMetadata.getLogs().forEach(mutator::putWal);
newTabletsFiles.get(newExtent).forEach(mutator::putFile);
@@ -221,6 +229,9 @@ public class UpdateTablets extends ManagerRepo {
var mutator =
tabletsMutator.mutateTablet(splitInfo.getOriginal()).requireOperation(opid)
.requireAbsentLocation().requireAbsentLogs();
+ Preconditions
+ .checkArgument(Objects.equals(tabletMetadata.getExtent().endRow(),
newExtent.endRow()));
+
mutator.putPrevEndRow(newExtent.prevEndRow());
newTabletsFiles.get(newExtent).forEach(mutator::putFile);
@@ -246,6 +257,31 @@ public class UpdateTablets extends ManagerRepo {
tabletMetadata.getSelectedFiles().getFateId());
}
+ // Remove any user compaction requested markers as the tablet may fall
outside the compaction
+ // range. The markers will be recreated if needed.
+
tabletMetadata.getUserCompactionsRequested().forEach(mutator::deleteUserCompactionRequested);
+
+ // scan entries are related to a hosted tablet, this tablet is not
hosted so can safely delete
+ // these
+ tabletMetadata.getScans().forEach(mutator::deleteScan);
+
+ if (tabletMetadata.getHostingRequested()) {
+ // The range of the tablet is changing, so lets delete the hosting
requested column in case
+ // this tablet does not actually need to be hosted.
+ mutator.deleteHostingRequested();
+ }
+
+ if (tabletMetadata.getSuspend() != null) {
+ // This no longer the exact tablet that was suspended. For consistency
should either delete
+ // the suspension marker OR add it to the new tablets. Choosing to
delete it.
+ mutator.deleteSuspension();
+ }
+
+ if (tabletMetadata.getLast() != null) {
+ // This is no longer the same tablet so lets delete the last location.
+ mutator.deleteLocation(tabletMetadata.getLast());
+ }
+
mutator.submit(tm -> false);
var result = tabletsMutator.process().get(splitInfo.getOriginal());
diff --git
a/server/manager/src/test/java/org/apache/accumulo/manager/tableOps/split/UpdateTabletsTest.java
b/server/manager/src/test/java/org/apache/accumulo/manager/tableOps/split/UpdateTabletsTest.java
index 313fc24891..baccd84680 100644
---
a/server/manager/src/test/java/org/apache/accumulo/manager/tableOps/split/UpdateTabletsTest.java
+++
b/server/manager/src/test/java/org/apache/accumulo/manager/tableOps/split/UpdateTabletsTest.java
@@ -18,17 +18,45 @@
*/
package org.apache.accumulo.manager.tableOps.split;
+import static org.easymock.EasyMock.mock;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.EnumSet;
+import java.util.List;
import java.util.Map;
+import java.util.OptionalLong;
import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.UUID;
+import org.apache.accumulo.core.client.admin.TabletAvailability;
import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.fate.FateId;
+import org.apache.accumulo.core.fate.FateInstanceType;
import org.apache.accumulo.core.metadata.ReferencedTabletFile;
import org.apache.accumulo.core.metadata.StoredTabletFile;
+import org.apache.accumulo.core.metadata.SuspendingTServer;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.CompactionMetadata;
import org.apache.accumulo.core.metadata.schema.DataFileValue;
+import org.apache.accumulo.core.metadata.schema.ExternalCompactionId;
+import org.apache.accumulo.core.metadata.schema.MetadataTime;
+import org.apache.accumulo.core.metadata.schema.SelectedFiles;
import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType;
+import org.apache.accumulo.core.metadata.schema.TabletOperationId;
+import org.apache.accumulo.core.metadata.schema.TabletOperationType;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.manager.Manager;
+import org.apache.accumulo.manager.split.Splitter;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.metadata.ConditionalTabletMutatorImpl;
import org.apache.accumulo.server.util.FileUtil;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.Text;
@@ -47,6 +75,33 @@ public class UpdateTabletsTest {
return new FileUtil.FileInfo(new Text(start), new Text(end));
}
+ /**
+ * This is a set of tablet metadata columns that the split code is known to
handle. The purpose of
+ * the set is to detect when a new tablet metadata column was added without
considering the
+ * implications for splitting tablets. For a column to be in this set it
means an Accumulo
+ * developer has determined that split code can handle that column OR has
opened an issue about
+ * handling it.
+ */
+ private static final Set<ColumnType> COLUMNS_HANDLED_BY_SPLIT =
+ EnumSet.of(ColumnType.TIME, ColumnType.LOGS, ColumnType.FILES,
ColumnType.PREV_ROW,
+ ColumnType.OPID, ColumnType.LOCATION, ColumnType.ECOMP,
ColumnType.SELECTED,
+ ColumnType.LOADED, ColumnType.USER_COMPACTION_REQUESTED,
ColumnType.MERGED,
+ ColumnType.LAST, ColumnType.SCANS, ColumnType.DIR,
ColumnType.CLONED, ColumnType.FLUSH_ID,
+ ColumnType.FLUSH_NONCE, ColumnType.SUSPEND, ColumnType.AVAILABILITY,
+ ColumnType.HOSTING_REQUESTED, ColumnType.COMPACTED);
+
+ /**
+ * The purpose of this test is to catch new tablet metadata columns that
were added w/o
+ * considering splitting tablets.
+ */
+ @Test
+ public void checkColumns() {
+ for (ColumnType columnType : ColumnType.values()) {
+ assertTrue(COLUMNS_HANDLED_BY_SPLIT.contains(columnType),
+ "The split code does not know how to handle " + columnType);
+ }
+ }
+
// When a tablet splits its files are partitioned among the new children
tablets. This test
// exercises the partitioning code.
@Test
@@ -115,4 +170,277 @@ public class UpdateTabletsTest {
}));
}
+
+ /**
+ * The purpose of this test is create tablet with as many columns in its
metadata set as possible
+ * and exercise the split code with that tablet.
+ */
+ @Test
+ public void testManyColumns() throws Exception {
+ TableId tableId = TableId.of("123");
+ KeyExtent origExtent = new KeyExtent(tableId, new Text("m"), null);
+
+ var newExtent1 = new KeyExtent(tableId, new Text("c"), null);
+ var newExtent2 = new KeyExtent(tableId, new Text("h"), new Text("c"));
+ var newExtent3 = new KeyExtent(tableId, new Text("m"), new Text("h"));
+
+ var file1 = newSTF(1);
+ var file2 = newSTF(2);
+ var file3 = newSTF(3);
+ var file4 = newSTF(4);
+
+ var loaded1 = newSTF(5);
+ var loaded2 = newSTF(6);
+
+ var flid1 = FateId.from(FateInstanceType.USER, 11L);
+ var flid2 = FateId.from(FateInstanceType.USER, 22L);
+ var loaded = Map.of(loaded1, flid1, loaded2, flid2);
+
+ var dfv1 = new DataFileValue(1000, 100, 20);
+ var dfv2 = new DataFileValue(500, 50, 20);
+ var dfv3 = new DataFileValue(4000, 400);
+ var dfv4 = new DataFileValue(2000, 200);
+
+ var tabletFiles = Map.of(file1, dfv1, file2, dfv2, file3, dfv3, file4,
dfv4);
+
+ var cid1 = ExternalCompactionId.generate(UUID.randomUUID());
+ var cid2 = ExternalCompactionId.generate(UUID.randomUUID());
+ var cid3 = ExternalCompactionId.generate(UUID.randomUUID());
+
+ var fateId = FateId.from(FateInstanceType.USER, 42L);
+ var opid = TabletOperationId.from(TabletOperationType.SPLITTING, fateId);
+ var tabletTime = MetadataTime.parse("L30");
+ var flushID = OptionalLong.of(40);
+ var availability = TabletAvailability.HOSTED;
+ var lastLocation = TabletMetadata.Location.last("1.2.3.4:1234",
"123456789");
+ var suspendingTServer = SuspendingTServer.fromValue(new
Value("1.2.3.4:5|56"));
+
+ String dir1 = "dir1";
+ String dir2 = "dir2";
+
+ Manager manager = EasyMock.mock(Manager.class);
+ ServerContext context = EasyMock.mock(ServerContext.class);
+ EasyMock.expect(manager.getContext()).andReturn(context).atLeastOnce();
+ Ample ample = EasyMock.mock(Ample.class);
+ EasyMock.expect(context.getAmple()).andReturn(ample).atLeastOnce();
+ Splitter splitter = EasyMock.mock(Splitter.class);
+ EasyMock.expect(splitter.getCachedFileInfo(tableId,
file1)).andReturn(newFileInfo("a", "z"));
+ EasyMock.expect(splitter.getCachedFileInfo(tableId,
file2)).andReturn(newFileInfo("a", "b"));
+ EasyMock.expect(splitter.getCachedFileInfo(tableId,
file3)).andReturn(newFileInfo("d", "f"));
+ EasyMock.expect(splitter.getCachedFileInfo(tableId,
file4)).andReturn(newFileInfo("d", "j"));
+ EasyMock.expect(manager.getSplitter()).andReturn(splitter).atLeastOnce();
+
+ // Setup the metadata for the tablet that is going to split, set as many
columns as possible on
+ // it.
+ TabletMetadata tabletMeta = EasyMock.mock(TabletMetadata.class);
+
EasyMock.expect(tabletMeta.getExtent()).andReturn(origExtent).atLeastOnce();
+ EasyMock.expect(tabletMeta.getOperationId()).andReturn(opid).atLeastOnce();
+ EasyMock.expect(tabletMeta.getLocation()).andReturn(null).atLeastOnce();
+ EasyMock.expect(tabletMeta.getLogs()).andReturn(List.of()).atLeastOnce();
+ EasyMock.expect(tabletMeta.hasMerged()).andReturn(false).atLeastOnce();
+ EasyMock.expect(tabletMeta.getCloned()).andReturn(null).atLeastOnce();
+ Map<ExternalCompactionId,CompactionMetadata> compactions =
EasyMock.mock(Map.class);
+ EasyMock.expect(compactions.keySet()).andReturn(Set.of(cid1, cid2,
cid3)).anyTimes();
+
EasyMock.expect(tabletMeta.getExternalCompactions()).andReturn(compactions).atLeastOnce();
+
EasyMock.expect(tabletMeta.getFilesMap()).andReturn(tabletFiles).atLeastOnce();
+ EasyMock.expect(tabletMeta.getFiles()).andReturn(tabletFiles.keySet());
+ SelectedFiles selectedFiles = EasyMock.mock(SelectedFiles.class);
+ EasyMock.expect(selectedFiles.getFateId()).andReturn(null);
+
EasyMock.expect(tabletMeta.getSelectedFiles()).andReturn(selectedFiles).atLeastOnce();
+ FateId ucfid1 = FateId.from(FateInstanceType.USER, 55L);
+ FateId ucfid2 = FateId.from(FateInstanceType.USER, 66L);
+
EasyMock.expect(tabletMeta.getUserCompactionsRequested()).andReturn(Set.of(ucfid1,
ucfid2))
+ .atLeastOnce();
+ FateId ucfid3 = FateId.from(FateInstanceType.USER, 77L);
+ EasyMock.expect(tabletMeta.getCompacted()).andReturn(Set.of(ucfid1,
ucfid3)).atLeastOnce();
+ EasyMock.expect(tabletMeta.getScans()).andReturn(List.of(file1,
file2)).atLeastOnce();
+ EasyMock.expect(tabletMeta.getTime()).andReturn(tabletTime).atLeastOnce();
+ EasyMock.expect(tabletMeta.getFlushId()).andReturn(flushID).atLeastOnce();
+
EasyMock.expect(tabletMeta.getTabletAvailability()).andReturn(availability).atLeastOnce();
+ EasyMock.expect(tabletMeta.getLoaded()).andReturn(loaded).atLeastOnce();
+
EasyMock.expect(tabletMeta.getHostingRequested()).andReturn(true).atLeastOnce();
+
EasyMock.expect(tabletMeta.getSuspend()).andReturn(suspendingTServer).atLeastOnce();
+
EasyMock.expect(tabletMeta.getLast()).andReturn(lastLocation).atLeastOnce();
+
+ EasyMock.expect(ample.readTablet(origExtent)).andReturn(tabletMeta);
+
+ Ample.ConditionalTabletsMutator tabletsMutator =
+ EasyMock.mock(Ample.ConditionalTabletsMutator.class);
+
EasyMock.expect(ample.conditionallyMutateTablets()).andReturn(tabletsMutator).atLeastOnce();
+
+ // Setup the mutator for creating the first new tablet
+ ConditionalTabletMutatorImpl tablet1Mutator =
EasyMock.mock(ConditionalTabletMutatorImpl.class);
+
EasyMock.expect(tablet1Mutator.requireAbsentTablet()).andReturn(tablet1Mutator);
+
EasyMock.expect(tablet1Mutator.putOperation(opid)).andReturn(tablet1Mutator);
+ EasyMock.expect(tablet1Mutator.putDirName(dir1)).andReturn(tablet1Mutator);
+
EasyMock.expect(tablet1Mutator.putTime(tabletTime)).andReturn(tablet1Mutator);
+
EasyMock.expect(tablet1Mutator.putFlushId(flushID.getAsLong())).andReturn(tablet1Mutator);
+ EasyMock.expect(tablet1Mutator.putPrevEndRow(newExtent1.prevEndRow()))
+ .andReturn(tablet1Mutator);
+
EasyMock.expect(tablet1Mutator.putCompacted(ucfid1)).andReturn(tablet1Mutator);
+
EasyMock.expect(tablet1Mutator.putCompacted(ucfid3)).andReturn(tablet1Mutator);
+
EasyMock.expect(tablet1Mutator.putTabletAvailability(availability)).andReturn(tablet1Mutator);
+ EasyMock.expect(tablet1Mutator.putBulkFile(loaded1.getTabletFile(), flid1))
+ .andReturn(tablet1Mutator);
+ EasyMock.expect(tablet1Mutator.putBulkFile(loaded2.getTabletFile(), flid2))
+ .andReturn(tablet1Mutator);
+ EasyMock.expect(tablet1Mutator.putFile(file1, new DataFileValue(333, 33,
20)))
+ .andReturn(tablet1Mutator);
+ EasyMock.expect(tablet1Mutator.putFile(file2,
dfv2)).andReturn(tablet1Mutator);
+ tablet1Mutator.submit(EasyMock.anyObject());
+ EasyMock.expectLastCall().once();
+
EasyMock.expect(tabletsMutator.mutateTablet(newExtent1)).andReturn(tablet1Mutator);
+
+ // Setup the mutator for creating the second new tablet
+ ConditionalTabletMutatorImpl tablet2Mutator =
EasyMock.mock(ConditionalTabletMutatorImpl.class);
+
EasyMock.expect(tablet2Mutator.requireAbsentTablet()).andReturn(tablet2Mutator);
+
EasyMock.expect(tablet2Mutator.putOperation(opid)).andReturn(tablet2Mutator);
+ EasyMock.expect(tablet2Mutator.putDirName(dir2)).andReturn(tablet2Mutator);
+
EasyMock.expect(tablet2Mutator.putTime(tabletTime)).andReturn(tablet2Mutator);
+
EasyMock.expect(tablet2Mutator.putFlushId(flushID.getAsLong())).andReturn(tablet2Mutator);
+ EasyMock.expect(tablet2Mutator.putPrevEndRow(newExtent2.prevEndRow()))
+ .andReturn(tablet2Mutator);
+
EasyMock.expect(tablet2Mutator.putCompacted(ucfid1)).andReturn(tablet2Mutator);
+
EasyMock.expect(tablet2Mutator.putCompacted(ucfid3)).andReturn(tablet2Mutator);
+
EasyMock.expect(tablet2Mutator.putTabletAvailability(availability)).andReturn(tablet2Mutator);
+ EasyMock.expect(tablet2Mutator.putBulkFile(loaded1.getTabletFile(), flid1))
+ .andReturn(tablet2Mutator);
+ EasyMock.expect(tablet2Mutator.putBulkFile(loaded2.getTabletFile(), flid2))
+ .andReturn(tablet2Mutator);
+ EasyMock.expect(tablet2Mutator.putFile(file1, new DataFileValue(333, 33,
20)))
+ .andReturn(tablet2Mutator);
+ EasyMock.expect(tablet2Mutator.putFile(file3,
dfv3)).andReturn(tablet2Mutator);
+ EasyMock.expect(tablet2Mutator.putFile(file4, new DataFileValue(1000,
100)))
+ .andReturn(tablet2Mutator);
+ tablet2Mutator.submit(EasyMock.anyObject());
+ EasyMock.expectLastCall().once();
+
EasyMock.expect(tabletsMutator.mutateTablet(newExtent2)).andReturn(tablet2Mutator);
+
+ // Setup the mutator for updating the existing tablet
+ ConditionalTabletMutatorImpl tablet3Mutator =
mock(ConditionalTabletMutatorImpl.class);
+
EasyMock.expect(tablet3Mutator.requireOperation(opid)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.requireAbsentLocation()).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.requireAbsentLogs()).andReturn(tablet3Mutator);
+ EasyMock.expect(tablet3Mutator.putPrevEndRow(newExtent3.prevEndRow()))
+ .andReturn(tablet3Mutator);
+ EasyMock.expect(tablet3Mutator.putFile(file1, new DataFileValue(333, 33,
20)))
+ .andReturn(tablet3Mutator);
+ EasyMock.expect(tablet3Mutator.putFile(file4, new DataFileValue(1000,
100)))
+ .andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteFile(file2)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteFile(file3)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteExternalCompaction(cid1)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteExternalCompaction(cid2)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteExternalCompaction(cid3)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteSelectedFiles()).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteUserCompactionRequested(ucfid1)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteUserCompactionRequested(ucfid2)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteScan(file1)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteScan(file2)).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteHostingRequested()).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteSuspension()).andReturn(tablet3Mutator);
+
EasyMock.expect(tablet3Mutator.deleteLocation(lastLocation)).andReturn(tablet3Mutator);
+ tablet3Mutator.submit(EasyMock.anyObject());
+ EasyMock.expectLastCall().once();
+
EasyMock.expect(tabletsMutator.mutateTablet(origExtent)).andReturn(tablet3Mutator);
+
+ // setup processing of conditional mutations
+ Ample.ConditionalResult cr =
EasyMock.niceMock(Ample.ConditionalResult.class);
+
EasyMock.expect(cr.getStatus()).andReturn(Ample.ConditionalResult.Status.ACCEPTED)
+ .atLeastOnce();
+ EasyMock.expect(tabletsMutator.process())
+ .andReturn(Map.of(newExtent1, cr, newExtent2, cr, origExtent,
cr)).atLeastOnce();
+ tabletsMutator.close();
+ EasyMock.expectLastCall().anyTimes();
+
+ EasyMock.replay(manager, context, ample, tabletMeta, splitter,
tabletsMutator, tablet1Mutator,
+ tablet2Mutator, tablet3Mutator, cr, compactions);
+ // Now we can actually test the split code that writes the new tablets
with a bunch columns in
+ // the original tablet
+ SortedSet<Text> splits = new TreeSet<>(List.of(newExtent1.endRow(),
newExtent2.endRow()));
+ UpdateTablets updateTablets =
+ new UpdateTablets(new SplitInfo(origExtent, splits), List.of(dir1,
dir2));
+ updateTablets.call(fateId, manager);
+
+ EasyMock.verify(manager, context, ample, tabletMeta, splitter,
tabletsMutator, tablet1Mutator,
+ tablet2Mutator, tablet3Mutator, cr, compactions);
+ }
+
+ @Test
+ public void testErrors() throws Exception {
+ TableId tableId = TableId.of("123");
+ KeyExtent origExtent = new KeyExtent(tableId, new Text("m"), null);
+
+ var fateId = FateId.from(FateInstanceType.USER, 42L);
+ var opid = TabletOperationId.from(TabletOperationType.SPLITTING, fateId);
+
+ // Test splitting a tablet with a location
+ var location = TabletMetadata.Location.future(new
TServerInstance("1.2.3.4:1234", 123456789L));
+ var tablet1 =
+
TabletMetadata.builder(origExtent).putOperation(opid).putLocation(location).build();
+ var e = assertThrows(IllegalStateException.class, () ->
testError(origExtent, tablet1, fateId));
+ assertTrue(e.getMessage().contains(location.toString()));
+
+ // Test splitting a tablet without an operation id set
+ var tablet2 = TabletMetadata.builder(origExtent).build(ColumnType.OPID);
+ e = assertThrows(IllegalStateException.class, () -> testError(origExtent,
tablet2, fateId));
+ assertTrue(e.getMessage().contains("does not have expected operation id
"));
+ assertTrue(e.getMessage().contains("null"));
+
+ // Test splitting a tablet with an unexpected operation id
+ var fateId2 = FateId.from(FateInstanceType.USER, 24L);
+ var opid2 = TabletOperationId.from(TabletOperationType.SPLITTING, fateId2);
+ var tablet3 =
TabletMetadata.builder(origExtent).putOperation(opid2).build();
+ e = assertThrows(IllegalStateException.class, () -> testError(origExtent,
tablet3, fateId));
+ assertTrue(e.getMessage().contains("does not have expected operation id
"));
+ assertTrue(e.getMessage().contains(opid2.toString()));
+
+ // Test splitting a tablet with walogs
+ var walog = LogEntry.fromPath("localhost+8020/" + UUID.randomUUID());
+ var tablet4 =
TabletMetadata.builder(origExtent).putOperation(opid).putWal(walog)
+ .build(ColumnType.LOCATION);
+ e = assertThrows(IllegalStateException.class, () -> testError(origExtent,
tablet4, fateId));
+ assertTrue(e.getMessage().contains("unexpectedly had walogs"));
+ assertTrue(e.getMessage().contains(walog.toString()));
+
+ // Test splitting tablet with merged marker
+ var tablet5 =
TabletMetadata.builder(origExtent).putOperation(opid).setMerged()
+ .build(ColumnType.LOCATION, ColumnType.LOGS);
+ e = assertThrows(IllegalStateException.class, () -> testError(origExtent,
tablet5, fateId));
+ assertTrue(e.getMessage().contains("unexpectedly has a merged"));
+
+ // Test splitting tablet with cloned marker
+ TabletMetadata tablet6 = EasyMock.mock(TabletMetadata.class);
+ EasyMock.expect(tablet6.getExtent()).andReturn(origExtent).anyTimes();
+ EasyMock.expect(tablet6.getOperationId()).andReturn(opid).anyTimes();
+ EasyMock.expect(tablet6.getLocation()).andReturn(null).anyTimes();
+ EasyMock.expect(tablet6.getLogs()).andReturn(List.of()).anyTimes();
+ EasyMock.expect(tablet6.hasMerged()).andReturn(false);
+ EasyMock.expect(tablet6.getCloned()).andReturn("OK").atLeastOnce();
+ EasyMock.replay(tablet6);
+ e = assertThrows(IllegalStateException.class, () -> testError(origExtent,
tablet6, fateId));
+ assertTrue(e.getMessage().contains("unexpectedly has a cloned"));
+ EasyMock.verify(tablet6);
+ }
+
+ private static void testError(KeyExtent origExtent, TabletMetadata tm1,
FateId fateId)
+ throws Exception {
+ Manager manager = EasyMock.mock(Manager.class);
+ ServerContext context = EasyMock.mock(ServerContext.class);
+ EasyMock.expect(manager.getContext()).andReturn(context).atLeastOnce();
+ Ample ample = EasyMock.mock(Ample.class);
+ EasyMock.expect(context.getAmple()).andReturn(ample).atLeastOnce();
+
+ EasyMock.expect(ample.readTablet(origExtent)).andReturn(tm1);
+
+ EasyMock.replay(manager, context, ample);
+ // Now we can actually test the split code that writes the new tablets
with a bunch columns in
+ // the original tablet
+ SortedSet<Text> splits = new TreeSet<>(List.of(new Text("c")));
+ UpdateTablets updateTablets =
+ new UpdateTablets(new SplitInfo(origExtent, splits), List.of("d1"));
+ updateTablets.call(fateId, manager);
+
+ EasyMock.verify(manager, context, ample);
+ }
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java
index 77d6ca6c1a..a94adfbc79 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java
@@ -20,6 +20,7 @@ package org.apache.accumulo.test.functional;
import static java.util.Collections.singletonMap;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.OPID_COLUMN;
import static org.apache.accumulo.core.util.LazySingletons.RANDOM;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -36,6 +37,7 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Random;
import java.util.Set;
+import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
@@ -79,6 +81,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Preconditions;
+import com.google.common.collect.MoreCollectors;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -393,4 +396,55 @@ public class SplitIT extends AccumuloClusterHarness {
assertEquals(1000, c.createScanner(tableName).stream().count());
}
}
+
+ /**
+ * Test attempting to split a tablet that has an unexpected column
+ */
+ @Test
+ public void testUnexpectedColumn() throws Exception {
+ try (AccumuloClient c =
Accumulo.newClient().from(getClientProps()).build()) {
+ String tableName = getUniqueNames(1)[0];
+ c.tableOperations().create(tableName);
+
+ var ctx = getServerContext();
+ var tableId = ctx.getTableId(tableName);
+ var extent = new KeyExtent(tableId, null, null);
+
+ // This column is not expected to be present
+ var tabletMutator = ctx.getAmple().mutateTablet(extent);
+ tabletMutator.setMerged();
+ tabletMutator.mutate();
+
+ var tabletMetadata =
ctx.getAmple().readTablets().forTable(tableId).saveKeyValues().build()
+ .stream().collect(MoreCollectors.onlyElement());
+ assertEquals(extent, tabletMetadata.getExtent());
+
+ tabletMetadata.getKeyValues();
+
+ // Split operation should fail because of the unexpected column.
+ var splits = new TreeSet<>(List.of(new Text("m")));
+ assertThrows(AccumuloException.class, () ->
c.tableOperations().addSplits(tableName, splits));
+ assertEquals(Set.of(),
Set.copyOf(c.tableOperations().listSplits(tableName)));
+
+ // The tablet should be left in a bad state, so simulate a manual cleanup
+ var tabletMetadata2 =
ctx.getAmple().readTablets().forTable(tableId).saveKeyValues().build()
+ .stream().collect(MoreCollectors.onlyElement());
+ assertEquals(extent, tabletMetadata.getExtent());
+
+ // tablet should have an operation id set, but nothing else changed
+ var kvCopy = new TreeMap<>(tabletMetadata2.getKeyValues());
+ assertTrue(kvCopy.keySet().removeIf(OPID_COLUMN::hasColumns));
+ assertEquals(tabletMetadata.getKeyValues(), kvCopy);
+
+ // remove the offending columns
+ tabletMutator = ctx.getAmple().mutateTablet(extent);
+ tabletMutator.deleteMerged();
+ tabletMutator.deleteOperation();
+ tabletMutator.mutate();
+
+ // after cleaning up the tablet metadata, should be able to split
+ c.tableOperations().addSplits(tableName, splits);
+ assertEquals(Set.of(new Text("m")),
Set.copyOf(c.tableOperations().listSplits(tableName)));
+ }
+ }
}
