From cc95cc3fed01ef3fd589ead91fb5bbd4a75a6379 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Sat, 24 Feb 2024 12:16:25 -0500 Subject: [PATCH 01/13] Move the tracking of unsplittable tablets to metadata table This adds a new column to store information for tracking unsplittable tablets in the metadata table instead of in memory. This information can be used by the tablet management iterator to know if a tablet needs to split and avoid unnecessarily trying to split a tablet that can't be split. The data stored includes a hash of the file set and the relevant config related to splits and if this changes then the iterator will try and split again. --- .../accumulo/core/metadata/schema/Ample.java | 4 + .../core/metadata/schema/MetadataSchema.java | 13 ++ .../core/metadata/schema/TabletMetadata.java | 17 ++- .../schema/TabletMetadataBuilder.java | 12 ++ .../metadata/schema/TabletMutatorBase.java | 13 ++ .../core/metadata/schema/TabletsMetadata.java | 4 + .../metadata/schema/UnSplittableMetadata.java | 142 ++++++++++++++++++ .../constraints/MetadataConstraints.java | 2 + .../state/TabletManagementIterator.java | 26 +++- .../accumulo/manager/split/SplitUtils.java | 12 ++ .../manager/tableOps/split/FindSplits.java | 27 +++- .../manager/tableOps/split/UpdateTablets.java | 6 + .../apache/accumulo/test/LargeSplitRowIT.java | 21 ++- 13 files changed, 286 insertions(+), 13 deletions(-) create mode 100644 core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java 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 d3f27a7f360..d85553f75e6 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 @@ -419,6 +419,10 @@ interface TabletUpdates { T putUserCompactionRequested(FateId fateId); T deleteUserCompactionRequested(FateId fateId); + + T setUnSplittable(UnSplittableMetadata unSplittableMeta); + + T deleteUnSplittable(); } interface TabletMutator extends TabletUpdates { diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java index 46cbfb7b069..b093a4bccf7 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java @@ -430,6 +430,19 @@ public static class UserCompactionRequestedColumnFamily { public static final Text NAME = new Text(STR_NAME); } + /** + * This family is used to track information needed for splits. Currently, the only thing stored + * is if the tablets are un-splittable based on the files the tablet and configuration related + * to splits. + */ + public static class SplitColumnFamily { + public static final String STR_NAME = "split"; + public static final Text NAME = new Text(STR_NAME); + public static final String UNSPLITTABLE_QUAL = "unsplittable"; + public static final ColumnFQ UNSPLITTABLE_COLUMN = + new ColumnFQ(NAME, new Text(UNSPLITTABLE_QUAL)); + } + // TODO when removing the Upgrader12to13 class in the upgrade package, also remove this class. public static class Upgrade12to13 { diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java index ccdb2acaecd..bc8ae712f10 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java @@ -70,6 +70,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.MergedColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SplitColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.UserCompactionRequestedColumnFamily; @@ -120,6 +121,7 @@ public class TabletMetadata { private boolean futureAndCurrentLocationSet = false; private Set compacted; private Set userCompactionsRequested; + private UnSplittableMetadata unSplittableMetadata; public static TabletMetadataBuilder builder(KeyExtent extent) { return new TabletMetadataBuilder(extent); @@ -150,7 +152,8 @@ public enum ColumnType { OPID, SELECTED, COMPACTED, - USER_COMPACTION_REQUESTED + USER_COMPACTION_REQUESTED, + UNSPLITTABLE } public static class Location { @@ -381,6 +384,11 @@ public boolean getHostingRequested() { return onDemandHostingRequested; } + public UnSplittableMetadata getUnSplittable() { + ensureFetched(ColumnType.UNSPLITTABLE); + return unSplittableMetadata; + } + @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE).append("tableId", tableId) @@ -545,6 +553,13 @@ public static > TabletMetadata convertRow(Iterator case UserCompactionRequestedColumnFamily.STR_NAME: userCompactionsRequestedBuilder.add(FateId.from(qual)); break; + case SplitColumnFamily.STR_NAME: + if (qual.equals(SplitColumnFamily.UNSPLITTABLE_QUAL)) { + te.unSplittableMetadata = UnSplittableMetadata.fromJson(val); + } else { + throw new IllegalStateException("Unexpected SplitColumnFamily qualifier: " + qual); + } + break; default: throw new IllegalStateException("Unexpected family " + fam); diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java index 55a21075999..e41b45c96e5 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java @@ -295,6 +295,18 @@ public TabletMetadataBuilder deleteUserCompactionRequested(FateId fateId) { throw new UnsupportedOperationException(); } + @Override + public TabletMetadataBuilder setUnSplittable(UnSplittableMetadata unSplittableMeta) { + fetched.add(ECOMP); + internalBuilder.setUnSplittable(unSplittableMeta); + return this; + } + + @Override + public TabletMetadataBuilder deleteUnSplittable() { + throw new UnsupportedOperationException(); + } + /** * @param extraFetched Anything that was put on the builder will automatically be added to the * fetched set. However, for the case where something was not put and it needs to be diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java index 190439ab2d9..8e77a76b907 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java @@ -44,6 +44,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.MergedColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SplitColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.UserCompactionRequestedColumnFamily; @@ -347,6 +348,18 @@ public T deleteUserCompactionRequested(FateId fateId) { return getThis(); } + @Override + public T setUnSplittable(UnSplittableMetadata unSplittableMeta) { + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unSplittableMeta.toJson())); + return getThis(); + } + + @Override + public T deleteUnSplittable() { + SplitColumnFamily.UNSPLITTABLE_COLUMN.putDelete(mutation); + return getThis(); + } + public void setCloseAfterMutate(AutoCloseable closeable) { this.closeAfterMutate = closeable; } diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java index 4fee556643e..9e61d9103cb 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java @@ -81,6 +81,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.MergedColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SplitColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.UserCompactionRequestedColumnFamily; import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; @@ -396,6 +397,9 @@ public Options fetch(ColumnType... colsToFetch) { case USER_COMPACTION_REQUESTED: families.add(UserCompactionRequestedColumnFamily.NAME); break; + case UNSPLITTABLE: + qualifiers.add(SplitColumnFamily.UNSPLITTABLE_COLUMN); + break; default: throw new IllegalArgumentException("Unknown col type " + colToFetch); } diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java new file mode 100644 index 00000000000..6a9eb92a647 --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java @@ -0,0 +1,142 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.core.metadata.schema; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.accumulo.core.util.LazySingletons.GSON; + +import java.util.Objects; +import java.util.Set; + +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.util.json.ByteArrayToBase64TypeAdapter; + +import com.google.common.base.Preconditions; +import com.google.common.hash.HashCode; +import com.google.common.hash.Hashing; +import com.google.gson.Gson; + +public class UnSplittableMetadata { + + private static final Gson GSON = ByteArrayToBase64TypeAdapter.createBase64Gson(); + + private final long splitThreshold; + private final long maxEndRowSize; + private final int maxFilesToOpen; + private final HashCode filesHash; + + public UnSplittableMetadata(long splitThreshold, long maxEndRowSize, int maxFilesToOpen, + Set files) { + this(splitThreshold, maxEndRowSize, maxFilesToOpen, caclulateFilesHash(files)); + } + + public UnSplittableMetadata(long splitThreshold, long maxEndRowSize, int maxFilesToOpen, + HashCode filesHash) { + this.splitThreshold = splitThreshold; + this.maxEndRowSize = maxEndRowSize; + this.maxFilesToOpen = maxFilesToOpen; + this.filesHash = Objects.requireNonNull(filesHash); + + Preconditions.checkState(splitThreshold > 0, "splitThreshold must be greater than 0"); + Preconditions.checkState(maxEndRowSize > 0, "maxEndRowSize must be greater than 0"); + Preconditions.checkState(maxFilesToOpen > 0, "maxFilesToOpen must be greater than 0"); + } + + public long getSplitThreshold() { + return splitThreshold; + } + + public long getMaxEndRowSize() { + return maxEndRowSize; + } + + public int getMaxFilesToOpen() { + return maxFilesToOpen; + } + + public HashCode getFilesHash() { + return filesHash; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + UnSplittableMetadata that = (UnSplittableMetadata) o; + return splitThreshold == that.splitThreshold && maxEndRowSize == that.maxEndRowSize + && maxFilesToOpen == that.maxFilesToOpen && Objects.equals(filesHash, that.filesHash); + } + + @Override + public int hashCode() { + return Objects.hash(splitThreshold, maxEndRowSize, maxFilesToOpen, filesHash); + } + + private static HashCode caclulateFilesHash(Set files) { + // Use static call to murmur3_128() so the seed is always the same + // Hashing.goodFastHash will seed with the current time, and we need the seed to be + // the same across restarts and instances + var hasher = Hashing.murmur3_128().newHasher(); + files.stream().map(StoredTabletFile::getNormalizedPathStr).sorted() + .forEach(path -> hasher.putString(path, UTF_8)); + return hasher.hash(); + } + + // This class is used to serialize and deserialize this class using GSon. Any changes to this + // class must consider persisted data. + private static class GSonData { + long splitThreshold; + long maxEndRowSize; + int maxFilesToOpen; + byte[] filesHash; + } + + public String toJson() { + GSonData jData = new GSonData(); + + jData.splitThreshold = splitThreshold; + jData.maxEndRowSize = maxEndRowSize; + jData.maxFilesToOpen = maxFilesToOpen; + jData.filesHash = filesHash.asBytes(); + + return GSON.toJson(jData); + } + + public static UnSplittableMetadata fromJson(String json) { + GSonData jData = GSON.fromJson(json, GSonData.class); + + return new UnSplittableMetadata(jData.splitThreshold, jData.maxEndRowSize, jData.maxFilesToOpen, + HashCode.fromBytes(jData.filesHash)); + } + + @Override + public String toString() { + return toJson(); + } + + public static UnSplittableMetadata toUnSplittable(long splitThreshold, long maxEndRowSize, + int maxFilesToOpen, Set files) { + return new UnSplittableMetadata(splitThreshold, maxEndRowSize, maxFilesToOpen, files); + } + +} diff --git a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java index abb1c7a27dd..f8e5b014189 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java +++ b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java @@ -52,6 +52,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.MergedColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SplitColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Upgrade12to13; @@ -95,6 +96,7 @@ public class MetadataConstraints implements Constraint { TabletColumnFamily.AVAILABILITY_COLUMN, TabletColumnFamily.REQUESTED_COLUMN, ServerColumnFamily.SELECTED_COLUMN, + SplitColumnFamily.UNSPLITTABLE_COLUMN, Upgrade12to13.COMPACT_COL); @SuppressWarnings("deprecation") diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java index df37df441cc..ca47dfaeed8 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java +++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java @@ -25,10 +25,12 @@ import java.util.HashSet; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; import java.util.SortedMap; import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.PluginEnvironment.Configuration; import org.apache.accumulo.core.client.ScannerBase; import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.ConfigurationCopy; @@ -47,6 +49,7 @@ import org.apache.accumulo.core.metadata.schema.DataFileValue; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletOperationType; +import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.core.spi.balancer.SimpleLoadBalancer; import org.apache.accumulo.core.spi.balancer.TabletBalancer; import org.apache.accumulo.core.spi.compaction.CompactionKind; @@ -69,7 +72,23 @@ public class TabletManagementIterator extends SkippingIterator { private TabletBalancer balancer; private static boolean shouldReturnDueToSplit(final TabletMetadata tm, - final long splitThreshold) { + final Configuration tableConfig) { + + final long splitThreshold = ConfigurationTypeHelper + .getFixedMemoryAsBytes(tableConfig.get(Property.TABLE_SPLIT_THRESHOLD.getKey())); + final long maxEndRowSize = ConfigurationTypeHelper + .getFixedMemoryAsBytes(tableConfig.get(Property.TABLE_MAX_END_ROW_SIZE.getKey())); + final int maxFilesToOpen = (int) ConfigurationTypeHelper.getFixedMemoryAsBytes( + tableConfig.get(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN.getKey())); + + // If current config/files match unsplittable metadata then we can't split + if (Optional + .ofNullable(tm.getUnSplittable()).filter(um -> um.equals(UnSplittableMetadata + .toUnSplittable(splitThreshold, maxEndRowSize, maxFilesToOpen, tm.getFiles()))) + .isPresent()) { + return false; + } + final long sumOfFileSizes = tm.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(); final boolean shouldSplit = sumOfFileSizes > splitThreshold; @@ -255,10 +274,7 @@ private void computeTabletManagementActions(final TabletMetadata tm, if (tm.getOperationId() == null && Collections.disjoint(REASONS_NOT_TO_SPLIT_OR_COMPACT, reasonsToReturnThisTablet)) { try { - final long splitThreshold = - ConfigurationTypeHelper.getFixedMemoryAsBytes(this.env.getPluginEnv() - .getConfiguration(tm.getTableId()).get(Property.TABLE_SPLIT_THRESHOLD.getKey())); - if (shouldReturnDueToSplit(tm, splitThreshold)) { + if (shouldReturnDueToSplit(tm, this.env.getPluginEnv().getConfiguration(tm.getTableId()))) { reasonsToReturnThisTablet.add(ManagementAction.NEEDS_SPLITTING); } // important to call this since reasonsToReturnThisTablet is passed to it diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java index bce10badb2a..e0a146be755 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java @@ -41,6 +41,7 @@ import org.apache.accumulo.core.metadata.TabletFile; import org.apache.accumulo.core.metadata.schema.DataFileValue; import org.apache.accumulo.core.metadata.schema.TabletMetadata; +import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.hadoop.fs.FileSystem; @@ -295,4 +296,15 @@ public static SortedSet findSplits(Iterable tabletIndexIterator, int return splits; } + + public static UnSplittableMetadata toUnSplittable(ServerContext context, + TabletMetadata tabletMetadata) { + var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); + var splitThreshold = tableConf.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); + var maxEndRowSize = tableConf.getAsBytes(Property.TABLE_MAX_END_ROW_SIZE); + int maxFilesToOpen = tableConf.getCount(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN); + + return new UnSplittableMetadata(splitThreshold, maxEndRowSize, maxFilesToOpen, + tabletMetadata.getFiles()); + } } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java index 695f5650cc3..13023334073 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java @@ -24,6 +24,7 @@ import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.fate.FateId; import org.apache.accumulo.core.fate.Repo; +import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.manager.Manager; import org.apache.accumulo.manager.split.SplitUtils; import org.apache.accumulo.manager.tableOps.ManagerRepo; @@ -31,6 +32,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Preconditions; + public class FindSplits extends ManagerRepo { private static final long serialVersionUID = 1L; @@ -45,7 +48,8 @@ public FindSplits(KeyExtent extent) { @Override public Repo call(FateId fateId, Manager manager) throws Exception { var extent = splitInfo.getOriginal(); - var tabletMetadata = manager.getContext().getAmple().readTablet(extent); + var ample = manager.getContext().getAmple(); + var tabletMetadata = ample.readTablet(extent); if (tabletMetadata == null) { log.trace("Table {} no longer exist, so not gonna try to find a split point for it", extent); @@ -58,6 +62,15 @@ public Repo call(FateId fateId, Manager manager) throws Exception { return null; } + if (tabletMetadata.getUnSplittable() != null) { + // The TabletManagementIterator should not be trying to split if the tablet was marked + // as unsplittable and the metadata hasn't changed + Preconditions.checkState( + !tabletMetadata.getUnSplittable() + .equals(SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata)), + "Unexpected split attempted on tablet %s that was marked as unsplittable", extent); + } + if (!tabletMetadata.getLogs().isEmpty()) { // This code is only called by system initiated splits, so if walogs are present it probably // makes sense to wait for the data in them to be written to a file before finding splits @@ -75,13 +88,17 @@ public Repo call(FateId fateId, Manager manager) throws Exception { if (splits.isEmpty()) { log.info("Tablet {} needs to split, but no split points could be found.", tabletMetadata.getExtent()); - // ELASTICITY_TODO record the fact that tablet is un-splittable in metadata table in a new - // column. Record the config used to reach this decision and a hash of the file. The tablet - // mgmt iterator can inspect this column and only try to split the tablet when something has - // changed. + + // TODO: Do we care about conditional mutations here? I don't think it is required because + // If something changes TabletManagementIterator will be comparing anyways and will detect it + UnSplittableMetadata unSplittableMeta = + SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata); + ample.mutateTablet(extent).setUnSplittable(unSplittableMeta).mutate(); + return null; } return new PreSplit(extent, splits); } + } 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 2fe3c563999..85d35bd4648 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 @@ -246,6 +246,12 @@ private void updateExistingTablet(FateId fateId, Manager manager, TabletMetadata tabletMetadata.getSelectedFiles().getFateId()); } + // Clean up any previous unsplittable marker + if (tabletMetadata.getUnSplittable() != null) { + mutator.deleteUnSplittable(); + log.debug("{} deleting stale unsplittable metadata from {}", fateId, newExtent); + } + mutator.submit(tm -> false); var result = tabletsMutator.process().get(splitInfo.getOriginal()); diff --git a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java index 68c21d67dd1..5776052e11e 100644 --- a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java +++ b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java @@ -44,8 +44,12 @@ import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Mutation; +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.metadata.schema.TabletMetadata; import org.apache.accumulo.core.security.Authorizations; +import org.apache.accumulo.manager.split.SplitUtils; import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; import org.apache.accumulo.test.functional.ConfigurableMacBase; import org.apache.accumulo.test.util.Wait; @@ -129,7 +133,8 @@ public void automaticSplitWith250Same() throws Exception { Property.TABLE_SPLIT_THRESHOLD.getKey(), "10K", Property.TABLE_FILE_COMPRESSION_TYPE.getKey(), "none", Property.TABLE_FILE_COMPRESSED_BLOCK_SIZE.getKey(), "64", - Property.TABLE_MAX_END_ROW_SIZE.getKey(), "1000" + Property.TABLE_MAX_END_ROW_SIZE.getKey(), "1000", + Property.TABLE_MAJC_RATIO.getKey(), "9999" ); // @formatter:on client.tableOperations().create(tableName, new NewTableConfiguration().setProperties(props)); @@ -155,7 +160,19 @@ public void automaticSplitWith250Same() throws Exception { // Flush the BatchWriter and table and sleep for a bit to make sure that there is enough time // for the table to split if need be. client.tableOperations().flush(tableName, new Text(), new Text("z"), true); - Thread.sleep(500L); + + // Wait for the tablet to be marked as unsplittable due to the system split running + TableId tableId = TableId.of(client.tableOperations().tableIdMap().get(tableName)); + Wait.waitFor(() -> { + TabletMetadata tm = + getServerContext().getAmple().readTablet(new KeyExtent(tableId, null, null)); + return tm.getUnSplittable() != null; + }, Wait.MAX_WAIT_MILLIS, 100); + + // Verify that the unsplittable column is read correctly + TabletMetadata tm = + getServerContext().getAmple().readTablet(new KeyExtent(tableId, null, null)); + assertEquals(tm.getUnSplittable(), SplitUtils.toUnSplittable(getServerContext(), tm)); // Make sure all the data that was put in the table is still correct int count = 0; From 4ef3f35490bdb8b12d2ab10e4344cea80a77fd5c Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 1 Mar 2024 17:19:14 -0500 Subject: [PATCH 02/13] Always return true to try and split if unsplittable metadata changes --- .../manager/state/TabletManagementIterator.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java index ca47dfaeed8..ebdd20f6ad4 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java +++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java @@ -25,7 +25,6 @@ import java.util.HashSet; import java.util.Map; import java.util.Map.Entry; -import java.util.Optional; import java.util.Set; import java.util.SortedMap; @@ -81,14 +80,16 @@ private static boolean shouldReturnDueToSplit(final TabletMetadata tm, final int maxFilesToOpen = (int) ConfigurationTypeHelper.getFixedMemoryAsBytes( tableConfig.get(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN.getKey())); - // If current config/files match unsplittable metadata then we can't split - if (Optional - .ofNullable(tm.getUnSplittable()).filter(um -> um.equals(UnSplittableMetadata - .toUnSplittable(splitThreshold, maxEndRowSize, maxFilesToOpen, tm.getFiles()))) - .isPresent()) { - return false; + // If the current computed metadata matches the current marker then we can't split, + // so we return false. If the marker is set but doesn't match then return true + // which gives a chance to clean up the marker and recheck. + var unsplittable = tm.getUnSplittable(); + if (unsplittable != null) { + return !unsplittable.equals(UnSplittableMetadata.toUnSplittable(splitThreshold, maxEndRowSize, + maxFilesToOpen, tm.getFiles())); } + // If unspilttable is not set at all then check if over split threshold final long sumOfFileSizes = tm.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(); final boolean shouldSplit = sumOfFileSizes > splitThreshold; From 1ee2c7be0920efdd07be82f84c108599e486bdb4 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 1 Mar 2024 18:01:46 -0500 Subject: [PATCH 03/13] Add unsplittable metadata to tostring --- .../apache/accumulo/core/metadata/schema/TabletMetadata.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java index bc8ae712f10..9c110ffd392 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java @@ -402,7 +402,8 @@ public String toString() { .append("onDemandHostingRequested", onDemandHostingRequested) .append("operationId", operationId).append("selectedFiles", selectedFiles) .append("futureAndCurrentLocationSet", futureAndCurrentLocationSet) - .append("userCompactionsRequested", userCompactionsRequested).toString(); + .append("userCompactionsRequested", userCompactionsRequested) + .append("unSplittableMetadata", unSplittableMetadata).toString(); } public SortedMap getKeyValues() { From c554dd4d21117c05046388ffb3bfdd326ca70d97 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Sat, 2 Mar 2024 09:31:39 -0500 Subject: [PATCH 04/13] Switch uFindSplits to use a conditional mutation for the unsplittable column --- .../manager/tableOps/split/FindSplits.java | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java index 13023334073..6053dd23b2e 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java @@ -18,13 +18,17 @@ */ package org.apache.accumulo.manager.tableOps.split; +import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.FILES; + import java.util.SortedSet; import java.util.TreeSet; +import java.util.function.Consumer; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.fate.FateId; import org.apache.accumulo.core.fate.Repo; -import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; +import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult; +import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult.Status; import org.apache.accumulo.manager.Manager; import org.apache.accumulo.manager.split.SplitUtils; import org.apache.accumulo.manager.tableOps.ManagerRepo; @@ -89,11 +93,25 @@ public Repo call(FateId fateId, Manager manager) throws Exception { log.info("Tablet {} needs to split, but no split points could be found.", tabletMetadata.getExtent()); - // TODO: Do we care about conditional mutations here? I don't think it is required because - // If something changes TabletManagementIterator will be comparing anyways and will detect it - UnSplittableMetadata unSplittableMeta = - SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata); - ample.mutateTablet(extent).setUnSplittable(unSplittableMeta).mutate(); + Consumer resultConsumer = result -> { + if (result.getStatus() == Status.REJECTED) { + log.debug("{} unsplittable metadata update for {} was rejected ", fateId, + result.getExtent()); + } + }; + + var unSplittableMeta = SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata); + + // With the current design we don't need to require the files to be the same + // for correctness as the TabletManagementIterator will detect the difference + // when computing the hash and retry a new split operation if there is not a match. + // But if we already know there's a change now, it would be more efficient to fail and + // retry the current fate op vs completing and having the iterator submit a new one. + try (var tabletsMutator = ample.conditionallyMutateTablets(resultConsumer)) { + var mutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation() + .requireSame(tabletMetadata, FILES).setUnSplittable(unSplittableMeta); + mutator.submit(tm -> unSplittableMeta.equals(tm.getUnSplittable())); + } return null; } From 3c72e5dc5c2557fb9d1685c34c166e0c3ef1d342 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Sat, 2 Mar 2024 10:22:10 -0500 Subject: [PATCH 05/13] Add unsplittable contstraint check/tests and update tablet metadata tests --- .../metadata/schema/UnSplittableMetadata.java | 6 +- .../metadata/schema/TabletMetadataTest.java | 58 ++++++++++++++++++- .../constraints/MetadataConstraints.java | 18 +++++- .../constraints/MetadataConstraintsTest.java | 55 ++++++++++++++++-- 4 files changed, 125 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java index 6a9eb92a647..11c00813791 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java @@ -53,9 +53,9 @@ public UnSplittableMetadata(long splitThreshold, long maxEndRowSize, int maxFile this.maxFilesToOpen = maxFilesToOpen; this.filesHash = Objects.requireNonNull(filesHash); - Preconditions.checkState(splitThreshold > 0, "splitThreshold must be greater than 0"); - Preconditions.checkState(maxEndRowSize > 0, "maxEndRowSize must be greater than 0"); - Preconditions.checkState(maxFilesToOpen > 0, "maxFilesToOpen must be greater than 0"); + Preconditions.checkArgument(splitThreshold > 0, "splitThreshold must be greater than 0"); + Preconditions.checkArgument(maxEndRowSize > 0, "maxEndRowSize must be greater than 0"); + Preconditions.checkArgument(maxFilesToOpen > 0, "maxFilesToOpen must be greater than 0"); } public long getSplitThreshold() { diff --git a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java index 8131a1a0df9..006ab2cd920 100644 --- a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java +++ b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java @@ -32,6 +32,7 @@ import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOCATION; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.MERGED; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.SUSPEND; +import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.UNSPLITTABLE; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.USER_COMPACTION_REQUESTED; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -69,6 +70,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.FutureLocationColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SplitColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.UserCompactionRequestedColumnFamily; @@ -131,6 +133,8 @@ public void testAllColumns() { MERGED_COLUMN.put(mutation, new Value()); mutation.put(UserCompactionRequestedColumnFamily.STR_NAME, FateId.from(type, 17).canonical(), ""); + var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2)); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toJson())); SortedMap rowMap = toRowMap(mutation); @@ -162,6 +166,7 @@ public void testAllColumns() { assertEquals(Set.of(sf1, sf2), Set.copyOf(tm.getScans())); assertTrue(tm.hasMerged()); assertTrue(tm.getUserCompactionsRequested().contains(FateId.from(type, 17))); + assertEquals(unsplittableMeta, tm.getUnSplittable()); } @Test @@ -339,7 +344,47 @@ public void testCompactionRequestedColumn() { } @Test - public void testUnkownColFamily() { + public void testUnsplittableColumn() { + KeyExtent extent = new KeyExtent(TableId.of("5"), new Text("df"), new Text("da")); + + StoredTabletFile sf1 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf")); + StoredTabletFile sf2 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf2.rf")); + StoredTabletFile sf3 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf3.rf")); + var unsplittableMeta = + UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2, sf3)); + + // Test with files + Mutation mutation = TabletColumnFamily.createPrevRowMutation(extent); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toJson())); + TabletMetadata tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), + EnumSet.of(UNSPLITTABLE), true, false); + assertEquals(unsplittableMeta, tm.getUnSplittable()); + assertEquals(unsplittableMeta.toJson(), tm.getUnSplittable().toJson()); + + // Test empty file set + unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of()); + mutation = TabletColumnFamily.createPrevRowMutation(extent); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toJson())); + tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), + EnumSet.of(UNSPLITTABLE), true, false); + assertEquals(unsplittableMeta, tm.getUnSplittable()); + assertEquals(unsplittableMeta.toJson(), tm.getUnSplittable().toJson()); + + // Column not set + mutation = TabletColumnFamily.createPrevRowMutation(extent); + tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), + EnumSet.of(UNSPLITTABLE), true, false); + assertNull(tm.getUnSplittable()); + + // Column not fetched + mutation = TabletColumnFamily.createPrevRowMutation(extent); + tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), + EnumSet.of(ColumnType.PREV_ROW), true, false); + assertThrows(IllegalStateException.class, tm::getUnSplittable); + } + + @Test + public void testUnknownColFamily() { KeyExtent extent = new KeyExtent(TableId.of("5"), new Text("df"), new Text("da")); Mutation mutation = TabletColumnFamily.createPrevRowMutation(extent); @@ -390,7 +435,7 @@ public void testBuilder() { .putFile(sf1, dfv1).putFile(sf2, dfv2).putBulkFile(rf1, FateId.from(type, 25)) .putBulkFile(rf2, FateId.from(type, 35)).putFlushId(27).putDirName("dir1").putScan(sf3) .putScan(sf4).putCompacted(FateId.from(type, 17)).putCompacted(FateId.from(type, 23)) - .build(ECOMP, HOSTING_REQUESTED, MERGED, USER_COMPACTION_REQUESTED); + .build(ECOMP, HOSTING_REQUESTED, MERGED, USER_COMPACTION_REQUESTED, UNSPLITTABLE); assertEquals(extent, tm.getExtent()); assertEquals(TabletAvailability.UNHOSTED, tm.getTabletAvailability()); @@ -406,6 +451,7 @@ public void testBuilder() { assertFalse(tm.getHostingRequested()); assertTrue(tm.getUserCompactionsRequested().isEmpty()); assertFalse(tm.hasMerged()); + assertNull(tm.getUnSplittable()); assertThrows(IllegalStateException.class, tm::getOperationId); assertThrows(IllegalStateException.class, tm::getSuspend); assertThrows(IllegalStateException.class, tm::getTime); @@ -429,6 +475,9 @@ public void testBuilder() { assertThrows(IllegalStateException.class, tm2::getHostingRequested); assertThrows(IllegalStateException.class, tm2::getSelectedFiles); assertThrows(IllegalStateException.class, tm2::getCompacted); + assertThrows(IllegalStateException.class, tm2::hasMerged); + assertThrows(IllegalStateException.class, tm2::getUserCompactionsRequested); + assertThrows(IllegalStateException.class, tm2::getUnSplittable); var ecid1 = ExternalCompactionId.generate(UUID.randomUUID()); CompactionMetadata ecm = new CompactionMetadata(Set.of(sf1, sf2), rf1, "cid1", @@ -438,11 +487,13 @@ public void testBuilder() { LogEntry le2 = LogEntry.fromPath("localhost+8020/" + UUID.randomUUID()); SelectedFiles selFiles = new SelectedFiles(Set.of(sf1, sf4), false, FateId.from(type, 159L)); + var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2)); TabletMetadata tm3 = TabletMetadata.builder(extent).putExternalCompaction(ecid1, ecm) .putSuspension(ser1, 45L).putTime(new MetadataTime(479, TimeType.LOGICAL)).putWal(le1) .putWal(le2).setHostingRequested().putSelectedFiles(selFiles).setMerged() - .putUserCompactionRequested(FateId.from(type, 159L)).build(); + .putUserCompactionRequested(FateId.from(type, 159L)).setUnSplittable(unsplittableMeta) + .build(); assertEquals(Set.of(ecid1), tm3.getExternalCompactions().keySet()); assertEquals(Set.of(sf1, sf2), tm3.getExternalCompactions().get(ecid1).getJobFiles()); @@ -458,6 +509,7 @@ public void testBuilder() { assertEquals(selFiles.getMetadataValue(), tm3.getSelectedFiles().getMetadataValue()); assertTrue(tm3.hasMerged()); assertTrue(tm3.getUserCompactionsRequested().contains(FateId.from(type, 159L))); + assertEquals(unsplittableMeta, tm3.getUnSplittable()); } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java index f8e5b014189..1c9b5587156 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java +++ b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java @@ -59,6 +59,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.UserCompactionRequestedColumnFamily; import org.apache.accumulo.core.metadata.schema.SelectedFiles; import org.apache.accumulo.core.metadata.schema.TabletOperationId; +import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.core.util.ColumnFQ; import org.apache.accumulo.core.util.cleaner.CleanerUtil; import org.apache.accumulo.server.ServerContext; @@ -272,11 +273,20 @@ public List check(Environment env, Mutation mutation) { } catch (RuntimeException e) { violations = addViolation(violations, 11); } - } else if (CompactedColumnFamily.NAME.equals(columnFamily) - || UserCompactionRequestedColumnFamily.NAME.equals(columnFamily)) { + } else if (CompactedColumnFamily.NAME.equals(columnFamily)) { if (!FateId.isFateId(columnQualifier.toString())) { violations = addViolation(violations, 13); } + } else if (UserCompactionRequestedColumnFamily.NAME.equals(columnFamily)) { + if (!FateId.isFateId(columnQualifier.toString())) { + violations = addViolation(violations, 14); + } + } else if (SplitColumnFamily.UNSPLITTABLE_COLUMN.equals(columnFamily, columnQualifier)) { + try { + UnSplittableMetadata.fromJson(new String(columnUpdate.getValue(), UTF_8)); + } catch (RuntimeException e) { + violations = addViolation(violations, 15); + } } else if (columnFamily.equals(BulkFileColumnFamily.NAME)) { if (!columnUpdate.isDeleted() && !checkedBulk) { /* @@ -437,6 +447,10 @@ public String getViolationDescription(short violationCode) { return "Invalid data file metadata format"; case 13: return "Invalid compacted column"; + case 14: + return "Invalid user compaction requested column"; + case 15: + return "Invalid unsplittable column"; } return null; } diff --git a/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java b/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java index 89e22a09de2..72074a5f285 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java @@ -19,8 +19,10 @@ package org.apache.accumulo.server.constraints; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertIterableEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.lang.reflect.Method; import java.util.Base64; @@ -43,9 +45,11 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SplitColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.UserCompactionRequestedColumnFamily; import org.apache.accumulo.core.metadata.schema.SelectedFiles; +import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.server.ServerContext; import org.apache.hadoop.fs.Path; import org.apache.hadoop.io.Text; @@ -493,17 +497,17 @@ public void testSelectedFiles() { @Test public void testCompacted() { - testFateCqValidation(CompactedColumnFamily.STR_NAME); + testFateCqValidation(CompactedColumnFamily.STR_NAME, (short) 13); } @Test public void testUserCompactionRequested() { - testFateCqValidation(UserCompactionRequestedColumnFamily.STR_NAME); + testFateCqValidation(UserCompactionRequestedColumnFamily.STR_NAME, (short) 14); } // Verify that columns that store a FateId in their CQ // validate and only allow a correctly formatted FateId - private void testFateCqValidation(String column) { + private void testFateCqValidation(String column, short violation) { MetadataConstraints mc = new MetadataConstraints(); Mutation m; List violations; @@ -519,7 +523,50 @@ private void testFateCqValidation(String column) { violations = mc.check(createEnv(), m); assertNotNull(violations); assertEquals(1, violations.size()); - assertEquals(Short.valueOf((short) 13), violations.get(0)); + assertEquals(violation, violations.get(0)); + } + + @Test + public void testUnsplittableColumn() { + MetadataConstraints mc = new MetadataConstraints(); + Mutation m; + List violations; + + StoredTabletFile sf1 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf")); + var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1)); + + m = new Mutation(new Text("0;foo")); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value(unsplittableMeta.toJson())); + violations = mc.check(createEnv(), m); + assertNull(violations); + + // Verify empty value not allowed + m = new Mutation(new Text("0;foo")); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value()); + violations = mc.check(createEnv(), m); + assertNotNull(violations); + assertEquals(2, violations.size()); + assertIterableEquals(List.of((short) 6, (short) 15), violations); + + // test invalid args + assertThrows(IllegalArgumentException.class, + () -> UnSplittableMetadata.toUnSplittable(-100, 110, 120, Set.of(sf1))); + assertThrows(IllegalArgumentException.class, + () -> UnSplittableMetadata.toUnSplittable(100, -110, 120, Set.of(sf1))); + assertThrows(IllegalArgumentException.class, + () -> UnSplittableMetadata.toUnSplittable(100, 110, -120, Set.of(sf1))); + assertThrows(NullPointerException.class, + () -> UnSplittableMetadata.toUnSplittable(100, 110, -120, null)); + + // Test metadata constraints validate invalid json (negative arg) + m = new Mutation(new Text("0;foo")); + unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1)); + var invalidJson = unsplittableMeta.toJson().replace("100", "-100"); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value(invalidJson)); + violations = mc.check(createEnv(), m); + assertNotNull(violations); + assertEquals(1, violations.size()); + assertEquals(Short.valueOf((short) 15), violations.get(0)); } // Encode a row how it would appear in Json From 3209df5dd485bb98cb400e89f7aff8d1fe8d48a7 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Sat, 2 Mar 2024 10:34:17 -0500 Subject: [PATCH 06/13] Fix unsplittable fetched --- .../accumulo/core/metadata/schema/TabletMetadataBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java index e41b45c96e5..fb79fc80665 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataBuilder.java @@ -36,6 +36,7 @@ import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.SELECTED; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.SUSPEND; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.TIME; +import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.UNSPLITTABLE; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.USER_COMPACTION_REQUESTED; import java.util.Arrays; @@ -297,7 +298,7 @@ public TabletMetadataBuilder deleteUserCompactionRequested(FateId fateId) { @Override public TabletMetadataBuilder setUnSplittable(UnSplittableMetadata unSplittableMeta) { - fetched.add(ECOMP); + fetched.add(UNSPLITTABLE); internalBuilder.setUnSplittable(unSplittableMeta); return this; } From cb0f9959de1d223d6d76fbb13ffed7cc5e144867 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Sat, 2 Mar 2024 13:18:55 -0500 Subject: [PATCH 07/13] Handle cleanup of unsplittlable marker on metadata change and we no longer need a split add tests --- .../accumulo/manager/split/SplitUtils.java | 11 +- .../manager/tableOps/split/FindSplits.java | 72 ++++++--- .../manager/tableOps/split/UpdateTablets.java | 2 +- .../apache/accumulo/test/LargeSplitRowIT.java | 140 +++++++++++++++++- 4 files changed, 198 insertions(+), 27 deletions(-) diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java index e0a146be755..7d72c260d10 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java @@ -198,8 +198,12 @@ public static int calculateDesiredSplits(long esitimatedSize, long splitThreshol } public static SortedSet findSplits(ServerContext context, TabletMetadata tabletMetadata) { - var estimatedSize = - tabletMetadata.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(); + return findSplits(context, tabletMetadata, + tabletMetadata.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum()); + } + + public static SortedSet findSplits(ServerContext context, TabletMetadata tabletMetadata, + long estimatedSize) { var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); var threshold = tableConf.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); var maxEndRowSize = tableConf.getAsBytes(Property.TABLE_MAX_END_ROW_SIZE); @@ -304,7 +308,8 @@ public static UnSplittableMetadata toUnSplittable(ServerContext context, var maxEndRowSize = tableConf.getAsBytes(Property.TABLE_MAX_END_ROW_SIZE); int maxFilesToOpen = tableConf.getCount(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN); - return new UnSplittableMetadata(splitThreshold, maxEndRowSize, maxFilesToOpen, + return UnSplittableMetadata.toUnSplittable(splitThreshold, maxEndRowSize, maxFilesToOpen, tabletMetadata.getFiles()); } + } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java index 6053dd23b2e..80d0fd3faac 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java @@ -20,18 +20,24 @@ import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.FILES; +import java.util.Optional; import java.util.SortedSet; import java.util.TreeSet; import java.util.function.Consumer; +import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.fate.FateId; import org.apache.accumulo.core.fate.Repo; import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult; import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult.Status; +import org.apache.accumulo.core.metadata.schema.DataFileValue; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; +import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.manager.Manager; import org.apache.accumulo.manager.split.SplitUtils; import org.apache.accumulo.manager.tableOps.ManagerRepo; +import org.apache.accumulo.server.ServerContext; import org.apache.hadoop.io.Text; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,6 +60,7 @@ public Repo call(FateId fateId, Manager manager) throws Exception { var extent = splitInfo.getOriginal(); var ample = manager.getContext().getAmple(); var tabletMetadata = ample.readTablet(extent); + Optional computedUnsplittable = Optional.empty(); if (tabletMetadata == null) { log.trace("Table {} no longer exist, so not gonna try to find a split point for it", extent); @@ -66,12 +73,13 @@ public Repo call(FateId fateId, Manager manager) throws Exception { return null; } + // The TabletManagementIterator should not be trying to split if the tablet was marked + // as unsplittable and the metadata hasn't changed so check that the metadata is different if (tabletMetadata.getUnSplittable() != null) { - // The TabletManagementIterator should not be trying to split if the tablet was marked - // as unsplittable and the metadata hasn't changed + computedUnsplittable = + Optional.of(SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata)); Preconditions.checkState( - !tabletMetadata.getUnSplittable() - .equals(SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata)), + !tabletMetadata.getUnSplittable().equals(computedUnsplittable.orElseThrow()), "Unexpected split attempted on tablet %s that was marked as unsplittable", extent); } @@ -83,16 +91,16 @@ public Repo call(FateId fateId, Manager manager) throws Exception { tabletMetadata.getLogs().size()); } - SortedSet splits = SplitUtils.findSplits(manager.getContext(), tabletMetadata); + var estimatedSize = + tabletMetadata.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(); + SortedSet splits = + SplitUtils.findSplits(manager.getContext(), tabletMetadata, estimatedSize); if (extent.endRow() != null) { splits.remove(extent.endRow()); } if (splits.isEmpty()) { - log.info("Tablet {} needs to split, but no split points could be found.", - tabletMetadata.getExtent()); - Consumer resultConsumer = result -> { if (result.getStatus() == Status.REJECTED) { log.debug("{} unsplittable metadata update for {} was rejected ", fateId, @@ -100,17 +108,34 @@ public Repo call(FateId fateId, Manager manager) throws Exception { } }; - var unSplittableMeta = SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata); - - // With the current design we don't need to require the files to be the same - // for correctness as the TabletManagementIterator will detect the difference - // when computing the hash and retry a new split operation if there is not a match. - // But if we already know there's a change now, it would be more efficient to fail and - // retry the current fate op vs completing and having the iterator submit a new one. try (var tabletsMutator = ample.conditionallyMutateTablets(resultConsumer)) { - var mutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation() - .requireSame(tabletMetadata, FILES).setUnSplittable(unSplittableMeta); - mutator.submit(tm -> unSplittableMeta.equals(tm.getUnSplittable())); + // Check if we still need to split. It's possible we don't if the unsplittable marker + // has already been previously set. This could happen in some scenarios such as + // a compaction that shrinks a previously unsplittable tablet below the threshold + // or if the threshold has been raised higher because the tablet management iterator + // will try and split any time the computed metadata changes. + if (stillNeedsSplit(manager.getContext(), tabletMetadata, estimatedSize)) { + log.info("Tablet {} needs to split, but no split points could be found.", + tabletMetadata.getExtent()); + var unSplittableMeta = computedUnsplittable + .orElseGet(() -> SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata)); + + // With the current design we don't need to require the files to be the same + // for correctness as the TabletManagementIterator will detect the difference + // when computing the hash and retry a new split operation if there is not a match. + // But if we already know there's a change now, it would be more efficient to fail and + // retry the current fate op vs completing and having the iterator submit a new one. + var mutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation() + .requireSame(tabletMetadata, FILES).setUnSplittable(unSplittableMeta); + mutator.submit(tm -> unSplittableMeta.equals(tm.getUnSplittable())); + } else { + // We no longer need to split so we can clear the marker. + log.info("Tablet {} no longer needs to split, deleting unsplittable marker.", + tabletMetadata.getExtent()); + var mutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation() + .requireSame(tabletMetadata, FILES).deleteUnSplittable(); + mutator.submit(tm -> tm.getUnSplittable() == null); + } } return null; @@ -119,4 +144,15 @@ public Repo call(FateId fateId, Manager manager) throws Exception { return new PreSplit(extent, splits); } + private boolean stillNeedsSplit(ServerContext context, TabletMetadata tabletMetadata, + long estimatedSize) { + if (tabletMetadata.getUnSplittable() != null) { + // Recheck threshold if existing marker exists + var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); + var splitThreshold = tableConf.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); + return estimatedSize > splitThreshold; + } + return true; + } + } 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 85d35bd4648..49f2af35850 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 @@ -249,7 +249,7 @@ private void updateExistingTablet(FateId fateId, Manager manager, TabletMetadata // Clean up any previous unsplittable marker if (tabletMetadata.getUnSplittable() != null) { mutator.deleteUnSplittable(); - log.debug("{} deleting stale unsplittable metadata from {}", fateId, newExtent); + log.debug("{} deleting unsplittable metadata from {} because of split", fateId, newExtent); } mutator.submit(tm -> false); diff --git a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java index 5776052e11e..3a52586e8c3 100644 --- a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java +++ b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java @@ -163,11 +163,9 @@ public void automaticSplitWith250Same() throws Exception { // Wait for the tablet to be marked as unsplittable due to the system split running TableId tableId = TableId.of(client.tableOperations().tableIdMap().get(tableName)); - Wait.waitFor(() -> { - TabletMetadata tm = - getServerContext().getAmple().readTablet(new KeyExtent(tableId, null, null)); - return tm.getUnSplittable() != null; - }, Wait.MAX_WAIT_MILLIS, 100); + Wait.waitFor(() -> getServerContext().getAmple() + .readTablet(new KeyExtent(tableId, null, null)).getUnSplittable() != null, + Wait.MAX_WAIT_MILLIS, 100); // Verify that the unsplittable column is read correctly TabletMetadata tm = @@ -259,6 +257,138 @@ public void automaticSplitLater() throws Exception { } } + @Test + @Timeout(60) + public void testUnsplittableColumn() throws Exception { + log.info("Unsplittable Column Test"); + try (AccumuloClient client = Accumulo.newClient().from(getClientProperties()).build()) { + // make a table and lower the configuration properties + // @formatter:off + var maxEndRow = 100; + Map props = Map.of( + Property.TABLE_SPLIT_THRESHOLD.getKey(), "1K", + Property.TABLE_FILE_COMPRESSION_TYPE.getKey(), "none", + Property.TABLE_FILE_COMPRESSED_BLOCK_SIZE.getKey(), "64", + Property.TABLE_MAX_END_ROW_SIZE.getKey(), "" + maxEndRow, + Property.TABLE_MAJC_RATIO.getKey(), "9999" + ); + // @formatter:on + + final String tableName = getUniqueNames(1)[0]; + client.tableOperations().create(tableName, new NewTableConfiguration().setProperties(props)); + + // Create a key for a table entry that is longer than the allowed size for an + // end row and fill this key with all m's except the last spot + byte[] data = new byte[maxEndRow + 1]; + Arrays.fill(data, 0, data.length - 2, (byte) 'm'); + + final int numOfMutations = 20; + try (BatchWriter batchWriter = client.createBatchWriter(tableName)) { + // Make the last place in the key different for every entry added to the table + for (int i = 0; i < numOfMutations; i++) { + data[data.length - 1] = (byte) i; + Mutation m = new Mutation(data); + m.put("cf", "cq", "value"); + batchWriter.addMutation(m); + } + } + // Flush the BatchWriter and table + client.tableOperations().flush(tableName, null, null, true); + + // Wait for the tablets to be marked as unsplittable due to the system split running + TableId tableId = TableId.of(client.tableOperations().tableIdMap().get(tableName)); + Wait.waitFor(() -> getServerContext().getAmple() + .readTablet(new KeyExtent(tableId, null, null)).getUnSplittable() != null, + Wait.MAX_WAIT_MILLIS, 100); + + // Verify that the unsplittable column is read correctly + TabletMetadata tm = + getServerContext().getAmple().readTablet(new KeyExtent(tableId, null, null)); + assertEquals(tm.getUnSplittable(), SplitUtils.toUnSplittable(getServerContext(), tm)); + + // Make sure no splits occurred in the table + assertTrue(client.tableOperations().listSplits(tableName).isEmpty()); + + // Bump max end row size and verify split occurs and unsplittable column is cleaned up + client.tableOperations().setProperty(tableName, Property.TABLE_MAX_END_ROW_SIZE.getKey(), + "500"); + + // Wait for splits to occur + assertTrue(client.tableOperations().listSplits(tableName).isEmpty()); + Wait.waitFor(() -> !client.tableOperations().listSplits(tableName).isEmpty(), + Wait.MAX_WAIT_MILLIS, 100); + + // Verify all tablets have no unsplittable metadata column + Wait.waitFor(() -> { + try (var tabletsMetadata = + getServerContext().getAmple().readTablets().forTable(tableId).build()) { + return tabletsMetadata.stream() + .allMatch(tabletMetadata -> tabletMetadata.getUnSplittable() == null); + } + }, Wait.MAX_WAIT_MILLIS, 100); + } + } + + // Test the unsplittable column is cleaned up if a previously marked unsplittable tablet + // no longer needs to be split + @Test + @Timeout(60) + public void testUnsplittableCleanup() throws Exception { + log.info("Unsplittable Column Cleanup"); + try (AccumuloClient client = Accumulo.newClient().from(getClientProperties()).build()) { + // make a table and lower the configuration properties + // @formatter:off + var maxEndRow = 100; + Map props = Map.of( + Property.TABLE_SPLIT_THRESHOLD.getKey(), "1K", + Property.TABLE_FILE_COMPRESSION_TYPE.getKey(), "none", + Property.TABLE_FILE_COMPRESSED_BLOCK_SIZE.getKey(), "64", + Property.TABLE_MAX_END_ROW_SIZE.getKey(), "" + maxEndRow, + Property.TABLE_MAJC_RATIO.getKey(), "9999" + ); + // @formatter:on + + final String tableName = getUniqueNames(1)[0]; + client.tableOperations().create(tableName, new NewTableConfiguration().setProperties(props)); + + // Create a key for a table entry that is longer than the allowed size for an + // end row and fill this key with all m's except the last spot + byte[] data = new byte[maxEndRow + 1]; + Arrays.fill(data, 0, data.length - 2, (byte) 'm'); + + final int numOfMutations = 20; + try (BatchWriter batchWriter = client.createBatchWriter(tableName)) { + // Make the last place in the key different for every entry added to the table + for (int i = 0; i < numOfMutations; i++) { + data[data.length - 1] = (byte) i; + Mutation m = new Mutation(data); + m.put("cf", "cq", "value"); + batchWriter.addMutation(m); + } + } + // Flush the BatchWriter and table + client.tableOperations().flush(tableName, null, null, true); + + // Wait for the tablets to be marked as unsplittable due to the system split running + TableId tableId = TableId.of(client.tableOperations().tableIdMap().get(tableName)); + Wait.waitFor(() -> getServerContext().getAmple() + .readTablet(new KeyExtent(tableId, null, null)).getUnSplittable() != null, + Wait.MAX_WAIT_MILLIS, 100); + + // Bump split threshold and verify marker is cleared + client.tableOperations().setProperty(tableName, Property.TABLE_SPLIT_THRESHOLD.getKey(), + "1M"); + + // Should still only be 1 tablet but no longer have a marker as it should be cleaned up + Wait.waitFor(() -> getServerContext().getAmple() + .readTablet(new KeyExtent(tableId, null, null)).getUnSplittable() == null, + Wait.MAX_WAIT_MILLIS, 100); + + // Should be no splits + assertTrue(client.tableOperations().listSplits(tableName).isEmpty()); + } + } + private void automaticSplit(AccumuloClient client, int max, int spacing) throws Exception { // make a table and lower the configuration properties // @formatter:off From 6db8417a35072e974df93f9de8884c6773158d5f Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 8 Mar 2024 07:13:08 -0500 Subject: [PATCH 08/13] Handle new column in UpdateTabletsTest --- .../accumulo/manager/tableOps/split/UpdateTabletsTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 baccd84680d..fb919793292 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 @@ -88,7 +88,7 @@ FileUtil.FileInfo newFileInfo(String start, String end) { 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); + ColumnType.HOSTING_REQUESTED, ColumnType.COMPACTED, ColumnType.UNSPLITTABLE); /** * The purpose of this test is to catch new tablet metadata columns that were added w/o @@ -261,6 +261,7 @@ public void testManyColumns() throws Exception { EasyMock.expect(tabletMeta.getHostingRequested()).andReturn(true).atLeastOnce(); EasyMock.expect(tabletMeta.getSuspend()).andReturn(suspendingTServer).atLeastOnce(); EasyMock.expect(tabletMeta.getLast()).andReturn(lastLocation).atLeastOnce(); + EasyMock.expect(tabletMeta.getUnSplittable()).andReturn(null).atLeastOnce(); EasyMock.expect(ample.readTablet(origExtent)).andReturn(tabletMeta); From 6c3358b5783b19ec6d73c7f7a121d0b0493c4333 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 8 Mar 2024 08:08:46 -0500 Subject: [PATCH 09/13] store unsplittable metadata as single hashcode, address other comments --- .../core/metadata/schema/TabletMetadata.java | 2 +- .../metadata/schema/TabletMutatorBase.java | 2 +- .../metadata/schema/UnSplittableMetadata.java | 96 ++++++------------- .../metadata/schema/TabletMetadataTest.java | 28 ++++-- .../constraints/MetadataConstraints.java | 2 +- .../constraints/MetadataConstraintsTest.java | 12 ++- .../accumulo/manager/split/SplitUtils.java | 11 ++- .../manager/tableOps/split/FindSplits.java | 3 + 8 files changed, 68 insertions(+), 88 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java index 9c110ffd392..893c401c7d7 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java @@ -556,7 +556,7 @@ public static > TabletMetadata convertRow(Iterator break; case SplitColumnFamily.STR_NAME: if (qual.equals(SplitColumnFamily.UNSPLITTABLE_QUAL)) { - te.unSplittableMetadata = UnSplittableMetadata.fromJson(val); + te.unSplittableMetadata = UnSplittableMetadata.toUnSplittable(val); } else { throw new IllegalStateException("Unexpected SplitColumnFamily qualifier: " + qual); } diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java index 8e77a76b907..9cc8143dc3b 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java @@ -350,7 +350,7 @@ public T deleteUserCompactionRequested(FateId fateId) { @Override public T setUnSplittable(UnSplittableMetadata unSplittableMeta) { - SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unSplittableMeta.toJson())); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unSplittableMeta.toBase64())); return getThis(); } diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java index 11c00813791..cf3fd6799b0 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java @@ -19,59 +19,28 @@ package org.apache.accumulo.core.metadata.schema; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.apache.accumulo.core.util.LazySingletons.GSON; +import java.util.Base64; import java.util.Objects; import java.util.Set; import org.apache.accumulo.core.metadata.StoredTabletFile; -import org.apache.accumulo.core.util.json.ByteArrayToBase64TypeAdapter; import com.google.common.base.Preconditions; import com.google.common.hash.HashCode; import com.google.common.hash.Hashing; -import com.google.gson.Gson; public class UnSplittableMetadata { - private static final Gson GSON = ByteArrayToBase64TypeAdapter.createBase64Gson(); - - private final long splitThreshold; - private final long maxEndRowSize; - private final int maxFilesToOpen; - private final HashCode filesHash; + private final HashCode hashOfSplitParameters; public UnSplittableMetadata(long splitThreshold, long maxEndRowSize, int maxFilesToOpen, Set files) { - this(splitThreshold, maxEndRowSize, maxFilesToOpen, caclulateFilesHash(files)); + this(caclulateSplitParamsHash(splitThreshold, maxEndRowSize, maxFilesToOpen, files)); } - public UnSplittableMetadata(long splitThreshold, long maxEndRowSize, int maxFilesToOpen, - HashCode filesHash) { - this.splitThreshold = splitThreshold; - this.maxEndRowSize = maxEndRowSize; - this.maxFilesToOpen = maxFilesToOpen; - this.filesHash = Objects.requireNonNull(filesHash); - - Preconditions.checkArgument(splitThreshold > 0, "splitThreshold must be greater than 0"); - Preconditions.checkArgument(maxEndRowSize > 0, "maxEndRowSize must be greater than 0"); - Preconditions.checkArgument(maxFilesToOpen > 0, "maxFilesToOpen must be greater than 0"); - } - - public long getSplitThreshold() { - return splitThreshold; - } - - public long getMaxEndRowSize() { - return maxEndRowSize; - } - - public int getMaxFilesToOpen() { - return maxFilesToOpen; - } - - public HashCode getFilesHash() { - return filesHash; + public UnSplittableMetadata(HashCode hashOfSplitParameters) { + this.hashOfSplitParameters = Objects.requireNonNull(hashOfSplitParameters); } @Override @@ -83,55 +52,46 @@ public boolean equals(Object o) { return false; } UnSplittableMetadata that = (UnSplittableMetadata) o; - return splitThreshold == that.splitThreshold && maxEndRowSize == that.maxEndRowSize - && maxFilesToOpen == that.maxFilesToOpen && Objects.equals(filesHash, that.filesHash); + return Objects.equals(hashOfSplitParameters, that.hashOfSplitParameters); } @Override public int hashCode() { - return Objects.hash(splitThreshold, maxEndRowSize, maxFilesToOpen, filesHash); + return Objects.hash(hashOfSplitParameters); + } + + @Override + public String toString() { + return toBase64(); + } + + public String toBase64() { + return Base64.getEncoder().encodeToString(hashOfSplitParameters.asBytes()); } - private static HashCode caclulateFilesHash(Set files) { + @SuppressWarnings("UnstableApiUsage") + private static HashCode caclulateSplitParamsHash(long splitThreshold, long maxEndRowSize, + int maxFilesToOpen, Set files) { + Preconditions.checkArgument(splitThreshold > 0, "splitThreshold must be greater than 0"); + Preconditions.checkArgument(maxEndRowSize > 0, "maxEndRowSize must be greater than 0"); + Preconditions.checkArgument(maxFilesToOpen > 0, "maxFilesToOpen must be greater than 0"); + // Use static call to murmur3_128() so the seed is always the same // Hashing.goodFastHash will seed with the current time, and we need the seed to be // the same across restarts and instances var hasher = Hashing.murmur3_128().newHasher(); + hasher.putLong(splitThreshold).putLong(maxEndRowSize).putInt(maxFilesToOpen); files.stream().map(StoredTabletFile::getNormalizedPathStr).sorted() .forEach(path -> hasher.putString(path, UTF_8)); return hasher.hash(); } - // This class is used to serialize and deserialize this class using GSon. Any changes to this - // class must consider persisted data. - private static class GSonData { - long splitThreshold; - long maxEndRowSize; - int maxFilesToOpen; - byte[] filesHash; + public static UnSplittableMetadata toUnSplittable(String base64HashOfSplitParameters) { + return toUnSplittable(Base64.getDecoder().decode(base64HashOfSplitParameters)); } - public String toJson() { - GSonData jData = new GSonData(); - - jData.splitThreshold = splitThreshold; - jData.maxEndRowSize = maxEndRowSize; - jData.maxFilesToOpen = maxFilesToOpen; - jData.filesHash = filesHash.asBytes(); - - return GSON.toJson(jData); - } - - public static UnSplittableMetadata fromJson(String json) { - GSonData jData = GSON.fromJson(json, GSonData.class); - - return new UnSplittableMetadata(jData.splitThreshold, jData.maxEndRowSize, jData.maxFilesToOpen, - HashCode.fromBytes(jData.filesHash)); - } - - @Override - public String toString() { - return toJson(); + public static UnSplittableMetadata toUnSplittable(byte[] hashOfSplitParameters) { + return new UnSplittableMetadata(HashCode.fromBytes(hashOfSplitParameters)); } public static UnSplittableMetadata toUnSplittable(long splitThreshold, long maxEndRowSize, diff --git a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java index 006ab2cd920..6e9ec6a5547 100644 --- a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java +++ b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java @@ -36,6 +36,7 @@ import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.USER_COMPACTION_REQUESTED; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -134,7 +135,7 @@ public void testAllColumns() { mutation.put(UserCompactionRequestedColumnFamily.STR_NAME, FateId.from(type, 17).canonical(), ""); var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2)); - SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toJson())); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toBase64())); SortedMap rowMap = toRowMap(mutation); @@ -350,25 +351,32 @@ public void testUnsplittableColumn() { StoredTabletFile sf1 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf")); StoredTabletFile sf2 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf2.rf")); StoredTabletFile sf3 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf3.rf")); - var unsplittableMeta = - UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2, sf3)); // Test with files + var unsplittableMeta1 = + UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2, sf3)); Mutation mutation = TabletColumnFamily.createPrevRowMutation(extent); - SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toJson())); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta1.toBase64())); TabletMetadata tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), EnumSet.of(UNSPLITTABLE), true, false); - assertEquals(unsplittableMeta, tm.getUnSplittable()); - assertEquals(unsplittableMeta.toJson(), tm.getUnSplittable().toJson()); + assertEquals(unsplittableMeta1, tm.getUnSplittable()); + assertEquals(unsplittableMeta1.hashCode(), tm.getUnSplittable().hashCode()); + assertEquals(unsplittableMeta1.toBase64(), tm.getUnSplittable().toBase64()); // Test empty file set - unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of()); + var unsplittableMeta2 = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of()); mutation = TabletColumnFamily.createPrevRowMutation(extent); - SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toJson())); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta2.toBase64())); tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), EnumSet.of(UNSPLITTABLE), true, false); - assertEquals(unsplittableMeta, tm.getUnSplittable()); - assertEquals(unsplittableMeta.toJson(), tm.getUnSplittable().toJson()); + assertEquals(unsplittableMeta2, tm.getUnSplittable()); + assertEquals(unsplittableMeta2.hashCode(), tm.getUnSplittable().hashCode()); + assertEquals(unsplittableMeta2.toBase64(), tm.getUnSplittable().toBase64()); + + // Make sure not equals works as well + assertNotEquals(unsplittableMeta1, unsplittableMeta2); + assertNotEquals(unsplittableMeta1.hashCode(), unsplittableMeta2.hashCode()); + assertNotEquals(unsplittableMeta1.toBase64(), unsplittableMeta2.toBase64()); // Column not set mutation = TabletColumnFamily.createPrevRowMutation(extent); diff --git a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java index 1c9b5587156..c6a30040a1f 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java +++ b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java @@ -283,7 +283,7 @@ public List check(Environment env, Mutation mutation) { } } else if (SplitColumnFamily.UNSPLITTABLE_COLUMN.equals(columnFamily, columnQualifier)) { try { - UnSplittableMetadata.fromJson(new String(columnUpdate.getValue(), UTF_8)); + UnSplittableMetadata.toUnSplittable(new String(columnUpdate.getValue(), UTF_8)); } catch (RuntimeException e) { violations = addViolation(violations, 15); } diff --git a/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java b/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java index 72074a5f285..276327eebff 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java @@ -536,7 +536,7 @@ public void testUnsplittableColumn() { var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1)); m = new Mutation(new Text("0;foo")); - SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value(unsplittableMeta.toJson())); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value(unsplittableMeta.toBase64())); violations = mc.check(createEnv(), m); assertNull(violations); @@ -556,13 +556,15 @@ public void testUnsplittableColumn() { assertThrows(IllegalArgumentException.class, () -> UnSplittableMetadata.toUnSplittable(100, 110, -120, Set.of(sf1))); assertThrows(NullPointerException.class, - () -> UnSplittableMetadata.toUnSplittable(100, 110, -120, null)); + () -> UnSplittableMetadata.toUnSplittable(100, 110, 120, null)); - // Test metadata constraints validate invalid json (negative arg) + // Test metadata constraints validate invalid hashcode m = new Mutation(new Text("0;foo")); unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1)); - var invalidJson = unsplittableMeta.toJson().replace("100", "-100"); - SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value(invalidJson)); + // partial hashcode is invalid + var invalidHashCode = + unsplittableMeta.toBase64().substring(0, unsplittableMeta.toBase64().length() - 1); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value(invalidHashCode)); violations = mc.check(createEnv(), m); assertNotNull(violations); assertEquals(1, violations.size()); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java index 7d72c260d10..9bdabfdcfd5 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java @@ -308,8 +308,15 @@ public static UnSplittableMetadata toUnSplittable(ServerContext context, var maxEndRowSize = tableConf.getAsBytes(Property.TABLE_MAX_END_ROW_SIZE); int maxFilesToOpen = tableConf.getCount(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN); - return UnSplittableMetadata.toUnSplittable(splitThreshold, maxEndRowSize, maxFilesToOpen, - tabletMetadata.getFiles()); + var unSplittableMetadata = UnSplittableMetadata.toUnSplittable(splitThreshold, maxEndRowSize, + maxFilesToOpen, tabletMetadata.getFiles()); + + log.trace( + "Created unsplittable metadata for tablet {}. splitThreshold: {}, maxEndRowSize:{}, maxFilesToOpen: {}, hashCode: {}", + tabletMetadata.getExtent(), splitThreshold, maxEndRowSize, maxFilesToOpen, + unSplittableMetadata); + + return unSplittableMetadata; } } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java index 80d0fd3faac..e88e103cfa3 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java @@ -89,6 +89,7 @@ public Repo call(FateId fateId, Manager manager) throws Exception { // points. log.debug("Not splitting {} because it has walogs {}", tabletMetadata.getExtent(), tabletMetadata.getLogs().size()); + return null; } var estimatedSize = @@ -125,6 +126,8 @@ public Repo call(FateId fateId, Manager manager) throws Exception { // when computing the hash and retry a new split operation if there is not a match. // But if we already know there's a change now, it would be more efficient to fail and // retry the current fate op vs completing and having the iterator submit a new one. + log.debug("Setting unsplittable metadata on tablet {}. hashCode: {}", + tabletMetadata.getExtent(), unSplittableMeta); var mutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation() .requireSame(tabletMetadata, FILES).setUnSplittable(unSplittableMeta); mutator.submit(tm -> unSplittableMeta.equals(tm.getUnSplittable())); From 50df68d4493004c3d8b55a5b5aa009f5101fd777 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 8 Mar 2024 09:01:21 -0500 Subject: [PATCH 10/13] FindSplits updates and memorize TM estimated file size sum --- .../core/metadata/schema/TabletMetadata.java | 13 ++++- .../org/apache/accumulo/core/util/Merge.java | 7 +-- .../state/TabletManagementIterator.java | 4 +- .../accumulo/manager/split/SplitUtils.java | 8 +-- .../manager/tableOps/split/FindSplits.java | 49 ++++++++++--------- 5 files changed, 43 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java index 893c401c7d7..f3a32af9b3b 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java @@ -83,6 +83,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -122,6 +124,7 @@ public class TabletMetadata { private Set compacted; private Set userCompactionsRequested; private UnSplittableMetadata unSplittableMetadata; + private Supplier fileSize; public static TabletMetadataBuilder builder(KeyExtent extent) { return new TabletMetadataBuilder(extent); @@ -319,6 +322,11 @@ public Map getFilesMap() { return files; } + public long getFileSize() { + ensureFetched(ColumnType.FILES); + return fileSize.get(); + } + public SelectedFiles getSelectedFiles() { ensureFetched(ColumnType.SELECTED); return selectedFiles; @@ -573,7 +581,10 @@ public static > TabletMetadata convertRow(Iterator te.availability = TabletAvailability.HOSTED; } - te.files = filesBuilder.build(); + var files = filesBuilder.build(); + te.files = files; + te.fileSize = + Suppliers.memoize(() -> files.values().stream().mapToLong(DataFileValue::getSize).sum()); te.loadedFiles = loadedFilesBuilder.build(); te.fetchedCols = fetchedColumns; te.scans = scansBuilder.build(); diff --git a/core/src/main/java/org/apache/accumulo/core/util/Merge.java b/core/src/main/java/org/apache/accumulo/core/util/Merge.java index 4eba179640f..c872086af7d 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/Merge.java +++ b/core/src/main/java/org/apache/accumulo/core/util/Merge.java @@ -37,7 +37,6 @@ import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.metadata.AccumuloTable; -import org.apache.accumulo.core.metadata.schema.DataFileValue; import org.apache.accumulo.core.metadata.schema.TabletsMetadata; import org.apache.accumulo.core.trace.TraceUtil; import org.apache.hadoop.io.Text; @@ -162,10 +161,8 @@ public void mergomatic(AccumuloClient client, String table, Text start, Text end .overRange(new KeyExtent(tableId, end, start).toMetaRange()).fetch(FILES, PREV_ROW) .build()) { - Iterator sizeIterator = tablets.stream().map(tm -> { - long size = tm.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(); - return new Size(tm.getExtent(), size); - }).iterator(); + Iterator sizeIterator = + tablets.stream().map(tm -> new Size(tm.getExtent(), tm.getFileSize())).iterator(); while (sizeIterator.hasNext()) { Size next = sizeIterator.next(); diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java index ebdd20f6ad4..3dd3c5c21ed 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java +++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java @@ -45,7 +45,6 @@ import org.apache.accumulo.core.manager.state.TabletManagement.ManagementAction; import org.apache.accumulo.core.manager.thrift.ManagerState; import org.apache.accumulo.core.metadata.TabletState; -import org.apache.accumulo.core.metadata.schema.DataFileValue; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletOperationType; import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; @@ -90,8 +89,7 @@ private static boolean shouldReturnDueToSplit(final TabletMetadata tm, } // If unspilttable is not set at all then check if over split threshold - final long sumOfFileSizes = - tm.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(); + final long sumOfFileSizes = tm.getFileSize(); final boolean shouldSplit = sumOfFileSizes > splitThreshold; LOG.trace("{} should split? sum: {}, threshold: {}, result: {}", tm.getExtent(), sumOfFileSizes, splitThreshold, shouldSplit); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java index 9bdabfdcfd5..c25a64ced48 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java @@ -39,7 +39,6 @@ import org.apache.accumulo.core.iteratorsImpl.system.MultiIterator; import org.apache.accumulo.core.metadata.StoredTabletFile; import org.apache.accumulo.core.metadata.TabletFile; -import org.apache.accumulo.core.metadata.schema.DataFileValue; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.server.ServerContext; @@ -198,12 +197,6 @@ public static int calculateDesiredSplits(long esitimatedSize, long splitThreshol } public static SortedSet findSplits(ServerContext context, TabletMetadata tabletMetadata) { - return findSplits(context, tabletMetadata, - tabletMetadata.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum()); - } - - public static SortedSet findSplits(ServerContext context, TabletMetadata tabletMetadata, - long estimatedSize) { var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); var threshold = tableConf.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); var maxEndRowSize = tableConf.getAsBytes(Property.TABLE_MAX_END_ROW_SIZE); @@ -212,6 +205,7 @@ public static SortedSet findSplits(ServerContext context, TabletMetadata t // anymore. int maxFilesToOpen = tableConf.getCount(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN); + var estimatedSize = tabletMetadata.getFileSize(); if (estimatedSize <= threshold) { return new TreeSet<>(); } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java index e88e103cfa3..2a8e55af8d5 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java @@ -31,7 +31,6 @@ import org.apache.accumulo.core.fate.Repo; import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult; import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult.Status; -import org.apache.accumulo.core.metadata.schema.DataFileValue; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.manager.Manager; @@ -42,8 +41,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Preconditions; - public class FindSplits extends ManagerRepo { private static final long serialVersionUID = 1L; @@ -73,14 +70,16 @@ public Repo call(FateId fateId, Manager manager) throws Exception { return null; } - // The TabletManagementIterator should not be trying to split if the tablet was marked + // The TabletManagementIterator should normally not be trying to split if the tablet was marked // as unsplittable and the metadata hasn't changed so check that the metadata is different if (tabletMetadata.getUnSplittable() != null) { computedUnsplittable = Optional.of(SplitUtils.toUnSplittable(manager.getContext(), tabletMetadata)); - Preconditions.checkState( - !tabletMetadata.getUnSplittable().equals(computedUnsplittable.orElseThrow()), - "Unexpected split attempted on tablet %s that was marked as unsplittable", extent); + if (tabletMetadata.getUnSplittable().equals(computedUnsplittable.orElseThrow())) { + log.debug("Not splitting {} because unsplittable metadata is present and did not change", + extent); + return null; + } } if (!tabletMetadata.getLogs().isEmpty()) { @@ -92,10 +91,7 @@ public Repo call(FateId fateId, Manager manager) throws Exception { return null; } - var estimatedSize = - tabletMetadata.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(); - SortedSet splits = - SplitUtils.findSplits(manager.getContext(), tabletMetadata, estimatedSize); + SortedSet splits = SplitUtils.findSplits(manager.getContext(), tabletMetadata); if (extent.endRow() != null) { splits.remove(extent.endRow()); @@ -110,12 +106,13 @@ public Repo call(FateId fateId, Manager manager) throws Exception { }; try (var tabletsMutator = ample.conditionallyMutateTablets(resultConsumer)) { - // Check if we still need to split. It's possible we don't if the unsplittable marker - // has already been previously set. This could happen in some scenarios such as - // a compaction that shrinks a previously unsplittable tablet below the threshold - // or if the threshold has been raised higher because the tablet management iterator - // will try and split any time the computed metadata changes. - if (stillNeedsSplit(manager.getContext(), tabletMetadata, estimatedSize)) { + // No split points were found, so we need to check if the tablet still + // needs to be split but is unsplittable, or if a split is not needed + + // Case 1: If a split is needed then set the unsplittable marker as no split + // points could be found so that we don't keep trying again until the + // split metadata is changed + if (stillNeedsSplit(manager.getContext(), tabletMetadata)) { log.info("Tablet {} needs to split, but no split points could be found.", tabletMetadata.getExtent()); var unSplittableMeta = computedUnsplittable @@ -131,13 +128,22 @@ public Repo call(FateId fateId, Manager manager) throws Exception { var mutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation() .requireSame(tabletMetadata, FILES).setUnSplittable(unSplittableMeta); mutator.submit(tm -> unSplittableMeta.equals(tm.getUnSplittable())); - } else { - // We no longer need to split so we can clear the marker. + + // Case 2: If the unsplittable marker has already been previously set, but we do not need + // to split then clear the marker. This could happen in some scenarios such as + // a compaction that shrinks a previously unsplittable tablet below the threshold + // or if the threshold has been raised higher because the tablet management iterator + // will try and split any time the computed metadata changes. + } else if (tabletMetadata.getUnSplittable() != null) { log.info("Tablet {} no longer needs to split, deleting unsplittable marker.", tabletMetadata.getExtent()); var mutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation() .requireSame(tabletMetadata, FILES).deleteUnSplittable(); mutator.submit(tm -> tm.getUnSplittable() == null); + // Case 3: The table config and/or set of files changed since the tablet mgmt iterator + // examined this tablet. + } else { + log.info("Tablet {} no longer needs to split, ignoring it.", tabletMetadata.getExtent()); } } @@ -147,13 +153,12 @@ public Repo call(FateId fateId, Manager manager) throws Exception { return new PreSplit(extent, splits); } - private boolean stillNeedsSplit(ServerContext context, TabletMetadata tabletMetadata, - long estimatedSize) { + private boolean stillNeedsSplit(ServerContext context, TabletMetadata tabletMetadata) { if (tabletMetadata.getUnSplittable() != null) { // Recheck threshold if existing marker exists var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); var splitThreshold = tableConf.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); - return estimatedSize > splitThreshold; + return tabletMetadata.getFileSize() > splitThreshold; } return true; } From 269baeda47ed99e3b4bb3b33d7dc07a1ba50e8f7 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 8 Mar 2024 11:27:17 -0500 Subject: [PATCH 11/13] Move SplitUtils and add keyextent to hash of split metadata --- .../core/metadata/schema/TabletMetadata.java | 3 ++ .../metadata/schema/UnSplittableMetadata.java | 44 ++++++++++++------- .../metadata/schema/TabletMetadataTest.java | 14 ++++-- .../state/TabletManagementIterator.java | 14 +++--- .../accumulo/server}/split/SplitUtils.java | 26 +++++++++-- .../constraints/MetadataConstraintsTest.java | 15 ++++--- .../server}/split/SplitUtilsTest.java | 2 +- .../manager/tableOps/split/FindSplits.java | 17 +------ .../apache/accumulo/test/LargeSplitRowIT.java | 2 +- 9 files changed, 84 insertions(+), 53 deletions(-) rename server/{manager/src/main/java/org/apache/accumulo/manager => base/src/main/java/org/apache/accumulo/server}/split/SplitUtils.java (90%) rename server/{manager/src/test/java/org/apache/accumulo/manager => base/src/test/java/org/apache/accumulo/server}/split/SplitUtilsTest.java (99%) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java index f3a32af9b3b..be7d2fb056d 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java @@ -322,6 +322,9 @@ public Map getFilesMap() { return files; } + /** + * @return the sum of the tablets files sizes + */ public long getFileSize() { ensureFetched(ColumnType.FILES); return fileSize.get(); diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java index cf3fd6799b0..f629ad67a92 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java @@ -20,10 +20,15 @@ import static java.nio.charset.StandardCharsets.UTF_8; +import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.UncheckedIOException; import java.util.Base64; import java.util.Objects; import java.util.Set; +import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.metadata.StoredTabletFile; import com.google.common.base.Preconditions; @@ -34,12 +39,12 @@ public class UnSplittableMetadata { private final HashCode hashOfSplitParameters; - public UnSplittableMetadata(long splitThreshold, long maxEndRowSize, int maxFilesToOpen, - Set files) { - this(caclulateSplitParamsHash(splitThreshold, maxEndRowSize, maxFilesToOpen, files)); + private UnSplittableMetadata(KeyExtent keyExtent, long splitThreshold, long maxEndRowSize, + int maxFilesToOpen, Set files) { + this(calculateSplitParamsHash(keyExtent, splitThreshold, maxEndRowSize, maxFilesToOpen, files)); } - public UnSplittableMetadata(HashCode hashOfSplitParameters) { + private UnSplittableMetadata(HashCode hashOfSplitParameters) { this.hashOfSplitParameters = Objects.requireNonNull(hashOfSplitParameters); } @@ -70,8 +75,8 @@ public String toBase64() { } @SuppressWarnings("UnstableApiUsage") - private static HashCode caclulateSplitParamsHash(long splitThreshold, long maxEndRowSize, - int maxFilesToOpen, Set files) { + private static HashCode calculateSplitParamsHash(KeyExtent keyExtent, long splitThreshold, + long maxEndRowSize, int maxFilesToOpen, Set files) { Preconditions.checkArgument(splitThreshold > 0, "splitThreshold must be greater than 0"); Preconditions.checkArgument(maxEndRowSize > 0, "maxEndRowSize must be greater than 0"); Preconditions.checkArgument(maxFilesToOpen > 0, "maxFilesToOpen must be greater than 0"); @@ -80,23 +85,32 @@ private static HashCode caclulateSplitParamsHash(long splitThreshold, long maxEn // Hashing.goodFastHash will seed with the current time, and we need the seed to be // the same across restarts and instances var hasher = Hashing.murmur3_128().newHasher(); - hasher.putLong(splitThreshold).putLong(maxEndRowSize).putInt(maxFilesToOpen); + hasher.putBytes(serializeKeyExtent(keyExtent)).putLong(splitThreshold).putLong(maxEndRowSize) + .putInt(maxFilesToOpen); files.stream().map(StoredTabletFile::getNormalizedPathStr).sorted() .forEach(path -> hasher.putString(path, UTF_8)); return hasher.hash(); } - public static UnSplittableMetadata toUnSplittable(String base64HashOfSplitParameters) { - return toUnSplittable(Base64.getDecoder().decode(base64HashOfSplitParameters)); + public static UnSplittableMetadata toUnSplittable(KeyExtent keyExtent, long splitThreshold, + long maxEndRowSize, int maxFilesToOpen, Set files) { + return new UnSplittableMetadata(keyExtent, splitThreshold, maxEndRowSize, maxFilesToOpen, + files); } - public static UnSplittableMetadata toUnSplittable(byte[] hashOfSplitParameters) { - return new UnSplittableMetadata(HashCode.fromBytes(hashOfSplitParameters)); + public static UnSplittableMetadata toUnSplittable(String base64HashOfSplitParameters) { + return new UnSplittableMetadata( + HashCode.fromBytes(Base64.getDecoder().decode(base64HashOfSplitParameters))); } - public static UnSplittableMetadata toUnSplittable(long splitThreshold, long maxEndRowSize, - int maxFilesToOpen, Set files) { - return new UnSplittableMetadata(splitThreshold, maxEndRowSize, maxFilesToOpen, files); + private static byte[] serializeKeyExtent(KeyExtent keyExtent) { + try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); + DataOutputStream dos = new DataOutputStream(baos)) { + keyExtent.writeTo(dos); + dos.close(); + return baos.toByteArray(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } } - } diff --git a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java index 6e9ec6a5547..f7875f2b09a 100644 --- a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java +++ b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java @@ -134,7 +134,8 @@ public void testAllColumns() { MERGED_COLUMN.put(mutation, new Value()); mutation.put(UserCompactionRequestedColumnFamily.STR_NAME, FateId.from(type, 17).canonical(), ""); - var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2)); + var unsplittableMeta = + UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf1, sf2)); SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta.toBase64())); SortedMap rowMap = toRowMap(mutation); @@ -148,6 +149,8 @@ public void testAllColumns() { assertEquals(extent, tm.getExtent()); assertEquals(Set.of(tf1, tf2), Set.copyOf(tm.getFiles())); assertEquals(Map.of(tf1, dfv1, tf2, dfv2), tm.getFilesMap()); + assertEquals(tm.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(), + tm.getFileSize()); assertEquals(6L, tm.getFlushId().getAsLong()); assertEquals(rowMap, tm.getKeyValues()); assertEquals(Map.of(new StoredTabletFile(bf1), fateId56L, new StoredTabletFile(bf2), fateId59L), @@ -354,7 +357,7 @@ public void testUnsplittableColumn() { // Test with files var unsplittableMeta1 = - UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2, sf3)); + UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf1, sf2, sf3)); Mutation mutation = TabletColumnFamily.createPrevRowMutation(extent); SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta1.toBase64())); TabletMetadata tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), @@ -364,7 +367,7 @@ public void testUnsplittableColumn() { assertEquals(unsplittableMeta1.toBase64(), tm.getUnSplittable().toBase64()); // Test empty file set - var unsplittableMeta2 = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of()); + var unsplittableMeta2 = UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of()); mutation = TabletColumnFamily.createPrevRowMutation(extent); SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta2.toBase64())); tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), @@ -450,6 +453,8 @@ public void testBuilder() { assertEquals(Location.future(ser1), tm.getLocation()); assertEquals(27L, tm.getFlushId().orElse(-1)); assertEquals(Map.of(sf1, dfv1, sf2, dfv2), tm.getFilesMap()); + assertEquals(tm.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum(), + tm.getFileSize()); assertEquals(Map.of(rf1.insert(), FateId.from(type, 25L), rf2.insert(), FateId.from(type, 35L)), tm.getLoaded()); assertEquals("dir1", tm.getDirName()); @@ -495,7 +500,8 @@ public void testBuilder() { LogEntry le2 = LogEntry.fromPath("localhost+8020/" + UUID.randomUUID()); SelectedFiles selFiles = new SelectedFiles(Set.of(sf1, sf4), false, FateId.from(type, 159L)); - var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1, sf2)); + var unsplittableMeta = + UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf1, sf2)); TabletMetadata tm3 = TabletMetadata.builder(extent).putExternalCompaction(ecid1, ecm) .putSuspension(ser1, 45L).putTime(new MetadataTime(479, TimeType.LOGICAL)).putWal(le1) diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java index 3dd3c5c21ed..4704f691c87 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java +++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java @@ -55,6 +55,7 @@ import org.apache.accumulo.server.fs.VolumeUtil; import org.apache.accumulo.server.iterators.TabletIteratorEnvironment; import org.apache.accumulo.server.manager.balancer.BalancerEnvironmentImpl; +import org.apache.accumulo.server.split.SplitUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -84,15 +85,14 @@ private static boolean shouldReturnDueToSplit(final TabletMetadata tm, // which gives a chance to clean up the marker and recheck. var unsplittable = tm.getUnSplittable(); if (unsplittable != null) { - return !unsplittable.equals(UnSplittableMetadata.toUnSplittable(splitThreshold, maxEndRowSize, - maxFilesToOpen, tm.getFiles())); + return !unsplittable.equals(UnSplittableMetadata.toUnSplittable(tm.getExtent(), + splitThreshold, maxEndRowSize, maxFilesToOpen, tm.getFiles())); } - // If unspilttable is not set at all then check if over split threshold - final long sumOfFileSizes = tm.getFileSize(); - final boolean shouldSplit = sumOfFileSizes > splitThreshold; - LOG.trace("{} should split? sum: {}, threshold: {}, result: {}", tm.getExtent(), sumOfFileSizes, - splitThreshold, shouldSplit); + // If unsplittable is not set at all then check if over split threshold + final boolean shouldSplit = SplitUtils.needsSplit(tableConfig, tm); + LOG.trace("{} should split? sum: {}, threshold: {}, result: {}", tm.getExtent(), + tm.getFileSize(), splitThreshold, shouldSplit); return shouldSplit; } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java b/server/base/src/main/java/org/apache/accumulo/server/split/SplitUtils.java similarity index 90% rename from server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java rename to server/base/src/main/java/org/apache/accumulo/server/split/SplitUtils.java index c25a64ced48..fcf4e2422fc 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/split/SplitUtils.java +++ b/server/base/src/main/java/org/apache/accumulo/server/split/SplitUtils.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.accumulo.manager.split; +package org.apache.accumulo.server.split; import java.io.IOException; import java.io.UncheckedIOException; @@ -29,6 +29,8 @@ import java.util.TreeSet; import java.util.function.Predicate; +import org.apache.accumulo.core.client.PluginEnvironment.Configuration; +import org.apache.accumulo.core.conf.ConfigurationTypeHelper; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; @@ -206,7 +208,7 @@ public static SortedSet findSplits(ServerContext context, TabletMetadata t int maxFilesToOpen = tableConf.getCount(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN); var estimatedSize = tabletMetadata.getFileSize(); - if (estimatedSize <= threshold) { + if (!needsSplit(context, tabletMetadata)) { return new TreeSet<>(); } @@ -295,6 +297,22 @@ public static SortedSet findSplits(Iterable tabletIndexIterator, int return splits; } + public static boolean needsSplit(ServerContext context, TabletMetadata tabletMetadata) { + var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); + var splitThreshold = tableConf.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); + return needsSplit(splitThreshold, tabletMetadata); + } + + public static boolean needsSplit(final Configuration tableConf, TabletMetadata tabletMetadata) { + var splitThreshold = ConfigurationTypeHelper + .getFixedMemoryAsBytes(tableConf.get(Property.TABLE_SPLIT_THRESHOLD.getKey())); + return needsSplit(splitThreshold, tabletMetadata); + } + + public static boolean needsSplit(long splitThreshold, TabletMetadata tabletMetadata) { + return tabletMetadata.getFileSize() > splitThreshold; + } + public static UnSplittableMetadata toUnSplittable(ServerContext context, TabletMetadata tabletMetadata) { var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); @@ -302,8 +320,8 @@ public static UnSplittableMetadata toUnSplittable(ServerContext context, var maxEndRowSize = tableConf.getAsBytes(Property.TABLE_MAX_END_ROW_SIZE); int maxFilesToOpen = tableConf.getCount(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN); - var unSplittableMetadata = UnSplittableMetadata.toUnSplittable(splitThreshold, maxEndRowSize, - maxFilesToOpen, tabletMetadata.getFiles()); + var unSplittableMetadata = UnSplittableMetadata.toUnSplittable(tabletMetadata.getExtent(), + splitThreshold, maxEndRowSize, maxFilesToOpen, tabletMetadata.getFiles()); log.trace( "Created unsplittable metadata for tablet {}. splitThreshold: {}, maxEndRowSize:{}, maxFilesToOpen: {}, hashCode: {}", diff --git a/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java b/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java index 276327eebff..5f3132acd37 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java @@ -33,6 +33,7 @@ import org.apache.accumulo.core.data.Mutation; import org.apache.accumulo.core.data.Range; 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.AccumuloTable; @@ -533,7 +534,8 @@ public void testUnsplittableColumn() { List violations; StoredTabletFile sf1 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf")); - var unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1)); + var unsplittableMeta = UnSplittableMetadata + .toUnSplittable(KeyExtent.fromMetaRow(new Text("0;foo")), 100, 110, 120, Set.of(sf1)); m = new Mutation(new Text("0;foo")); SplitColumnFamily.UNSPLITTABLE_COLUMN.put(m, new Value(unsplittableMeta.toBase64())); @@ -549,18 +551,19 @@ public void testUnsplittableColumn() { assertIterableEquals(List.of((short) 6, (short) 15), violations); // test invalid args + KeyExtent extent = KeyExtent.fromMetaRow(new Text("0;foo")); assertThrows(IllegalArgumentException.class, - () -> UnSplittableMetadata.toUnSplittable(-100, 110, 120, Set.of(sf1))); + () -> UnSplittableMetadata.toUnSplittable(extent, -100, 110, 120, Set.of(sf1))); assertThrows(IllegalArgumentException.class, - () -> UnSplittableMetadata.toUnSplittable(100, -110, 120, Set.of(sf1))); + () -> UnSplittableMetadata.toUnSplittable(extent, 100, -110, 120, Set.of(sf1))); assertThrows(IllegalArgumentException.class, - () -> UnSplittableMetadata.toUnSplittable(100, 110, -120, Set.of(sf1))); + () -> UnSplittableMetadata.toUnSplittable(extent, 100, 110, -120, Set.of(sf1))); assertThrows(NullPointerException.class, - () -> UnSplittableMetadata.toUnSplittable(100, 110, 120, null)); + () -> UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, null)); // Test metadata constraints validate invalid hashcode m = new Mutation(new Text("0;foo")); - unsplittableMeta = UnSplittableMetadata.toUnSplittable(100, 110, 120, Set.of(sf1)); + unsplittableMeta = UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf1)); // partial hashcode is invalid var invalidHashCode = unsplittableMeta.toBase64().substring(0, unsplittableMeta.toBase64().length() - 1); diff --git a/server/manager/src/test/java/org/apache/accumulo/manager/split/SplitUtilsTest.java b/server/base/src/test/java/org/apache/accumulo/server/split/SplitUtilsTest.java similarity index 99% rename from server/manager/src/test/java/org/apache/accumulo/manager/split/SplitUtilsTest.java rename to server/base/src/test/java/org/apache/accumulo/server/split/SplitUtilsTest.java index 80258e4914e..e8281e4c081 100644 --- a/server/manager/src/test/java/org/apache/accumulo/manager/split/SplitUtilsTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/split/SplitUtilsTest.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.accumulo.manager.split; +package org.apache.accumulo.server.split; import static java.util.stream.Collectors.toCollection; import static java.util.stream.Collectors.toList; diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java index 2a8e55af8d5..faf3212d25d 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java @@ -25,18 +25,15 @@ import java.util.TreeSet; import java.util.function.Consumer; -import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.fate.FateId; import org.apache.accumulo.core.fate.Repo; import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult; import org.apache.accumulo.core.metadata.schema.Ample.ConditionalResult.Status; -import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.manager.Manager; -import org.apache.accumulo.manager.split.SplitUtils; import org.apache.accumulo.manager.tableOps.ManagerRepo; -import org.apache.accumulo.server.ServerContext; +import org.apache.accumulo.server.split.SplitUtils; import org.apache.hadoop.io.Text; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -112,7 +109,7 @@ public Repo call(FateId fateId, Manager manager) throws Exception { // Case 1: If a split is needed then set the unsplittable marker as no split // points could be found so that we don't keep trying again until the // split metadata is changed - if (stillNeedsSplit(manager.getContext(), tabletMetadata)) { + if (SplitUtils.needsSplit(manager.getContext(), tabletMetadata)) { log.info("Tablet {} needs to split, but no split points could be found.", tabletMetadata.getExtent()); var unSplittableMeta = computedUnsplittable @@ -153,14 +150,4 @@ public Repo call(FateId fateId, Manager manager) throws Exception { return new PreSplit(extent, splits); } - private boolean stillNeedsSplit(ServerContext context, TabletMetadata tabletMetadata) { - if (tabletMetadata.getUnSplittable() != null) { - // Recheck threshold if existing marker exists - var tableConf = context.getTableConfiguration(tabletMetadata.getTableId()); - var splitThreshold = tableConf.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); - return tabletMetadata.getFileSize() > splitThreshold; - } - return true; - } - } diff --git a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java index 3a52586e8c3..788a09c0e18 100644 --- a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java +++ b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java @@ -49,8 +49,8 @@ import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.security.Authorizations; -import org.apache.accumulo.manager.split.SplitUtils; import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.server.split.SplitUtils; import org.apache.accumulo.test.functional.ConfigurableMacBase; import org.apache.accumulo.test.util.Wait; import org.apache.hadoop.conf.Configuration; From efe30445ad1e69151bdac4e93ac57f05d05d8e4e Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 8 Mar 2024 12:58:50 -0500 Subject: [PATCH 12/13] update testUnsplittableCleanup() to test unsplittable marker when all keys have the same endrow --- .../apache/accumulo/test/LargeSplitRowIT.java | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java index 788a09c0e18..666cdd7da1d 100644 --- a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java +++ b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java @@ -338,12 +338,10 @@ public void testUnsplittableCleanup() throws Exception { try (AccumuloClient client = Accumulo.newClient().from(getClientProperties()).build()) { // make a table and lower the configuration properties // @formatter:off - var maxEndRow = 100; Map props = Map.of( Property.TABLE_SPLIT_THRESHOLD.getKey(), "1K", Property.TABLE_FILE_COMPRESSION_TYPE.getKey(), "none", Property.TABLE_FILE_COMPRESSED_BLOCK_SIZE.getKey(), "64", - Property.TABLE_MAX_END_ROW_SIZE.getKey(), "" + maxEndRow, Property.TABLE_MAJC_RATIO.getKey(), "9999" ); // @formatter:on @@ -351,41 +349,53 @@ public void testUnsplittableCleanup() throws Exception { final String tableName = getUniqueNames(1)[0]; client.tableOperations().create(tableName, new NewTableConfiguration().setProperties(props)); - // Create a key for a table entry that is longer than the allowed size for an - // end row and fill this key with all m's except the last spot - byte[] data = new byte[maxEndRow + 1]; - Arrays.fill(data, 0, data.length - 2, (byte) 'm'); + byte[] data = new byte[100]; + Arrays.fill(data, 0, data.length - 1, (byte) 'm'); + // Write enough data that will cause a split. The row is not too large for a split + // but all the rows are the same so tablets won't be able to split except for + // the last tablet (null end row) final int numOfMutations = 20; try (BatchWriter batchWriter = client.createBatchWriter(tableName)) { // Make the last place in the key different for every entry added to the table for (int i = 0; i < numOfMutations; i++) { - data[data.length - 1] = (byte) i; Mutation m = new Mutation(data); - m.put("cf", "cq", "value"); + m.put("cf", "cq" + i, "value"); batchWriter.addMutation(m); } } // Flush the BatchWriter and table client.tableOperations().flush(tableName, null, null, true); - // Wait for the tablets to be marked as unsplittable due to the system split running TableId tableId = TableId.of(client.tableOperations().tableIdMap().get(tableName)); - Wait.waitFor(() -> getServerContext().getAmple() - .readTablet(new KeyExtent(tableId, null, null)).getUnSplittable() != null, - Wait.MAX_WAIT_MILLIS, 100); + + // Wait for a tablet to be marked as unsplittable due to the system split running + // There is enough data to split more than once so at least one tablet should be marked + // as unsplittable due to the same end row for all keys after the default tablet is split + Wait.waitFor(() -> { + try (var tabletsMetadata = + getServerContext().getAmple().readTablets().forTable(tableId).build()) { + return tabletsMetadata.stream().anyMatch(tm -> tm.getUnSplittable() != null); + } + }, Wait.MAX_WAIT_MILLIS, 100); + + var splits = client.tableOperations().listSplits(tableName); // Bump split threshold and verify marker is cleared client.tableOperations().setProperty(tableName, Property.TABLE_SPLIT_THRESHOLD.getKey(), "1M"); - // Should still only be 1 tablet but no longer have a marker as it should be cleaned up - Wait.waitFor(() -> getServerContext().getAmple() - .readTablet(new KeyExtent(tableId, null, null)).getUnSplittable() == null, - Wait.MAX_WAIT_MILLIS, 100); + // All tablets should now be cleared of the unsplittable marker, and we should have the + // same number of splits as before + Wait.waitFor(() -> { + try (var tabletsMetadata = + getServerContext().getAmple().readTablets().forTable(tableId).build()) { + return tabletsMetadata.stream().allMatch(tm -> tm.getUnSplittable() == null); + } + }, Wait.MAX_WAIT_MILLIS, 100); - // Should be no splits - assertTrue(client.tableOperations().listSplits(tableName).isEmpty()); + // no more splits should have happened + assertEquals(splits, client.tableOperations().listSplits(tableName)); } } From 12ee73ebc5ecc244681fc49e5528045f330ba995 Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Fri, 8 Mar 2024 16:44:54 -0500 Subject: [PATCH 13/13] Address latest comments --- .../metadata/schema/UnSplittableMetadata.java | 2 +- .../metadata/schema/TabletMetadataTest.java | 58 +++++++++++++++---- .../manager/tableOps/split/FindSplits.java | 2 +- .../tableOps/split/UpdateTabletsTest.java | 6 +- .../apache/accumulo/test/LargeSplitRowIT.java | 18 +++++- 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java index f629ad67a92..5c53c82952d 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UnSplittableMetadata.java @@ -87,7 +87,7 @@ private static HashCode calculateSplitParamsHash(KeyExtent keyExtent, long split var hasher = Hashing.murmur3_128().newHasher(); hasher.putBytes(serializeKeyExtent(keyExtent)).putLong(splitThreshold).putLong(maxEndRowSize) .putInt(maxFilesToOpen); - files.stream().map(StoredTabletFile::getNormalizedPathStr).sorted() + files.stream().map(StoredTabletFile::getMetadata).sorted() .forEach(path -> hasher.putString(path, UTF_8)); return hasher.hash(); } diff --git a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java index f7875f2b09a..e2fc8b8167a 100644 --- a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java +++ b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java @@ -36,7 +36,6 @@ import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.USER_COMPACTION_REQUESTED; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -54,6 +53,7 @@ import org.apache.accumulo.core.client.admin.TimeType; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.data.Range; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.dataImpl.KeyExtent; @@ -354,6 +354,9 @@ public void testUnsplittableColumn() { StoredTabletFile sf1 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf")); StoredTabletFile sf2 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf2.rf")); StoredTabletFile sf3 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf3.rf")); + // Same path as sf4 but with a range + StoredTabletFile sf4 = + StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf3.rf"), new Range("a", "b")); // Test with files var unsplittableMeta1 = @@ -362,9 +365,7 @@ public void testUnsplittableColumn() { SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta1.toBase64())); TabletMetadata tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), EnumSet.of(UNSPLITTABLE), true, false); - assertEquals(unsplittableMeta1, tm.getUnSplittable()); - assertEquals(unsplittableMeta1.hashCode(), tm.getUnSplittable().hashCode()); - assertEquals(unsplittableMeta1.toBase64(), tm.getUnSplittable().toBase64()); + assertUnsplittable(unsplittableMeta1, tm.getUnSplittable(), true); // Test empty file set var unsplittableMeta2 = UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of()); @@ -372,14 +373,23 @@ public void testUnsplittableColumn() { SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta2.toBase64())); tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), EnumSet.of(UNSPLITTABLE), true, false); - assertEquals(unsplittableMeta2, tm.getUnSplittable()); - assertEquals(unsplittableMeta2.hashCode(), tm.getUnSplittable().hashCode()); - assertEquals(unsplittableMeta2.toBase64(), tm.getUnSplittable().toBase64()); + assertUnsplittable(unsplittableMeta2, tm.getUnSplittable(), true); // Make sure not equals works as well - assertNotEquals(unsplittableMeta1, unsplittableMeta2); - assertNotEquals(unsplittableMeta1.hashCode(), unsplittableMeta2.hashCode()); - assertNotEquals(unsplittableMeta1.toBase64(), unsplittableMeta2.toBase64()); + assertUnsplittable(unsplittableMeta1, unsplittableMeta2, false); + + // Test with ranges + // use sf4 which includes sf4 instead of sf3 which has a range + var unsplittableMeta3 = + UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf1, sf2, sf4)); + mutation = TabletColumnFamily.createPrevRowMutation(extent); + SplitColumnFamily.UNSPLITTABLE_COLUMN.put(mutation, new Value(unsplittableMeta3.toBase64())); + tm = TabletMetadata.convertRow(toRowMap(mutation).entrySet().iterator(), + EnumSet.of(UNSPLITTABLE), true, false); + assertUnsplittable(unsplittableMeta3, tm.getUnSplittable(), true); + + // make sure not equals when all the file paths are equal but one has a range + assertUnsplittable(unsplittableMeta1, unsplittableMeta3, false); // Column not set mutation = TabletColumnFamily.createPrevRowMutation(extent); @@ -394,6 +404,34 @@ public void testUnsplittableColumn() { assertThrows(IllegalStateException.class, tm::getUnSplittable); } + @Test + public void testUnsplittableWithRange() { + KeyExtent extent = new KeyExtent(TableId.of("5"), new Text("df"), new Text("da")); + + // Files with same path and different ranges + StoredTabletFile sf1 = StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf")); + StoredTabletFile sf2 = + StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf"), new Range("a", "b")); + StoredTabletFile sf3 = + StoredTabletFile.of(new Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf"), new Range("a", "d")); + + var meta1 = UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf1)); + var meta2 = UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf2)); + var meta3 = UnSplittableMetadata.toUnSplittable(extent, 100, 110, 120, Set.of(sf3)); + + // compare each against the others to make sure not equal + assertUnsplittable(meta1, meta2, false); + assertUnsplittable(meta1, meta3, false); + assertUnsplittable(meta2, meta3, false); + } + + private void assertUnsplittable(UnSplittableMetadata meta1, UnSplittableMetadata meta2, + boolean equal) { + assertEquals(equal, meta1.equals(meta2)); + assertEquals(equal, meta1.hashCode() == meta2.hashCode()); + assertEquals(equal, meta1.toBase64().equals(meta2.toBase64())); + } + @Test public void testUnknownColFamily() { KeyExtent extent = new KeyExtent(TableId.of("5"), new Text("df"), new Text("da")); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java index faf3212d25d..7e3acd8d7f9 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java @@ -140,7 +140,7 @@ public Repo call(FateId fateId, Manager manager) throws Exception { // Case 3: The table config and/or set of files changed since the tablet mgmt iterator // examined this tablet. } else { - log.info("Tablet {} no longer needs to split, ignoring it.", tabletMetadata.getExtent()); + log.debug("Tablet {} no longer needs to split, ignoring it.", tabletMetadata.getExtent()); } } 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 fb919793292..226bc530346 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 @@ -52,6 +52,7 @@ 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.metadata.schema.UnSplittableMetadata; import org.apache.accumulo.core.tabletserver.log.LogEntry; import org.apache.accumulo.manager.Manager; import org.apache.accumulo.manager.split.Splitter; @@ -261,7 +262,9 @@ public void testManyColumns() throws Exception { EasyMock.expect(tabletMeta.getHostingRequested()).andReturn(true).atLeastOnce(); EasyMock.expect(tabletMeta.getSuspend()).andReturn(suspendingTServer).atLeastOnce(); EasyMock.expect(tabletMeta.getLast()).andReturn(lastLocation).atLeastOnce(); - EasyMock.expect(tabletMeta.getUnSplittable()).andReturn(null).atLeastOnce(); + UnSplittableMetadata usm = + UnSplittableMetadata.toUnSplittable(origExtent, 1000, 1001, 1002, tabletFiles.keySet()); + EasyMock.expect(tabletMeta.getUnSplittable()).andReturn(usm).atLeastOnce(); EasyMock.expect(ample.readTablet(origExtent)).andReturn(tabletMeta); @@ -341,6 +344,7 @@ public void testManyColumns() throws Exception { EasyMock.expect(tablet3Mutator.deleteHostingRequested()).andReturn(tablet3Mutator); EasyMock.expect(tablet3Mutator.deleteSuspension()).andReturn(tablet3Mutator); EasyMock.expect(tablet3Mutator.deleteLocation(lastLocation)).andReturn(tablet3Mutator); + EasyMock.expect(tablet3Mutator.deleteUnSplittable()).andReturn(tablet3Mutator); tablet3Mutator.submit(EasyMock.anyObject()); EasyMock.expectLastCall().once(); EasyMock.expect(tabletsMutator.mutateTablet(origExtent)).andReturn(tablet3Mutator); diff --git a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java index 666cdd7da1d..b810707cba0 100644 --- a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java +++ b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java @@ -304,11 +304,27 @@ public void testUnsplittableColumn() throws Exception { // Verify that the unsplittable column is read correctly TabletMetadata tm = getServerContext().getAmple().readTablet(new KeyExtent(tableId, null, null)); - assertEquals(tm.getUnSplittable(), SplitUtils.toUnSplittable(getServerContext(), tm)); + var unsplittable = tm.getUnSplittable(); + assertEquals(unsplittable, SplitUtils.toUnSplittable(getServerContext(), tm)); // Make sure no splits occurred in the table assertTrue(client.tableOperations().listSplits(tableName).isEmpty()); + // Bump the value for max end row by 1, we should still not be able to split but this should + // trigger an update to the unsplittable metadata value + client.tableOperations().setProperty(tableName, Property.TABLE_MAX_END_ROW_SIZE.getKey(), + "101"); + + // wait for the unsplittable marker to be set to a new value due to the property change + Wait.waitFor(() -> { + var updatedUnsplittable = getServerContext().getAmple() + .readTablet(new KeyExtent(tableId, null, null)).getUnSplittable(); + return updatedUnsplittable != null && !updatedUnsplittable.equals(unsplittable); + }, Wait.MAX_WAIT_MILLIS, 100); + // recheck with the computed meta is correct after property update + tm = getServerContext().getAmple().readTablet(new KeyExtent(tableId, null, null)); + assertEquals(tm.getUnSplittable(), SplitUtils.toUnSplittable(getServerContext(), tm)); + // Bump max end row size and verify split occurs and unsplittable column is cleaned up client.tableOperations().setProperty(tableName, Property.TABLE_MAX_END_ROW_SIZE.getKey(), "500");