You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2025/01/11 00:58:35 UTC
[PR] Avoids holding tablet metadata for compaction reservation [accumulo]
keith-turner opened a new pull request, #5247:
URL: https://github.com/apache/accumulo/pull/5247
This change fixes #5188. Unfortunately it touches a lot of code because of cascading dependencies in the code. It would be difficult to break this into a smaller commit. These changes do reduce some of those dependencies though.
There are two major advantages after this change. First tablet metadata is no longer kept in memory for queued compactions. Second the tablet metadata is no longer read during compaction reservation. Before this change the following would happen.
1. TGW would find a tablet to compact and enqueue compaction jobs+tablet metadata.
2. Eventually when a compactor requested a job it would yank job+tablet metadata off the queue.
3. To reserve the compaction a lot of complex analysis was done in the coordinator and then a conditional mutation was submitted. The conditional mutation would require all data involved in the complex analysis to be the same.
4. If the conditional mutation failed then the coordinator would reread the tablet metadata and go back to step 3.
After this change the following happens in the code.
1. TGW would find a tablet to compact and enqueue a new class called ResolvedCompactionJob. This new class takes the compaction job and tablet metadata and computes all information needed for the compaction later. The TabletMetadata object is no longer refrenced by this class after the constructor returns. So this class will use much less memory on the queue for the case when tablet have lots of files.
2. Eventually when a compactor requested a job it would yank a ResolvedCompactionJob off the queue.
3. All of the complex analysis to determine if a compaction can start is now done in the conditional mutation instead of in the coordinator. To enable this, new functionality was added to Ample including the new TabletMetadataCheck interface, TabletMetadataCheckIterator, and the CompactionReservationCheck class. With these changes its now super easy to write a conditional check that will do arbitrary analysis of TabletMetadata prior to committing a mutation.
4. Since the analysis is done in the conditional mutation there is no longer a need to retry. If the mutation fails then we know the compaction can not run.
The following were some supporting changes that had to be made.
* Took methods for encoding KeyExtent as base64 from TabletManagementParameters and moved the KeyExtent because this was needed in the new TabletMetadataCheckIterator.
* Move TabletMetadata out of the compaction queues, which made those more independent but was a big change. The main change here is that instead of adding `TabletMetadata, List<CompactionJob>` to the compaction queue now `KeyExtent, List<CompactionJob>` is added. This required changing the test for these classes and the code that interacts with them in CompactionCoordinator creating lots of diff.
* Moved CompactionCoordinatorTest.testCanReserve() to CompactionReservationCheckTest.testCanReserve() and changed the code to work with the new CompactionReservationCheck class. These are test of the complex logic that used to run in the coordinator and now runs in the tablet server as part of a conditional mutation.
* Removed conditional checks from Ample related to compactions that were no longer used after these changes. There was code for requiring a set of files not not be compacting.
The new TabletMetadataCheck functionality of ample could be used to simplify other conditional mutation checks of tablet metadata. It made the compaction reservation code much simpler and easier to understand. This code could be further improved if #5244 were implemented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1912238930
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/TabletMetadataCheckIterator.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.server.metadata.iterators;
+
+import static org.apache.accumulo.core.util.LazySingletons.GSON;
+import static org.apache.accumulo.server.metadata.iterators.ColumnFamilyTransformationIterator.getTabletRow;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.accumulo.core.classloader.ClassLoaderUtil;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.data.ByteSequence;
+import org.apache.accumulo.core.data.Condition;
+import org.apache.accumulo.core.data.Key;
+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.iterators.IteratorAdapter;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletMetadataCheck;
+import org.apache.accumulo.server.metadata.ConditionalTabletMutatorImpl;
+import org.apache.hadoop.io.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Preconditions;
+
+public class TabletMetadataCheckIterator implements SortedKeyValueIterator<Key,Value> {
+
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataCheckIterator.class);
+
+ private TabletMetadataCheck check;
+ private KeyExtent expectedExtent;
+
+ private Key startKey;
+ private Value topValue;
+ private SortedKeyValueIterator<Key,Value> source;
+
+ private static final String CHECK_CLASS_KEY = "checkClass";
+ private static final String CHECK_DATA_KEY = "checkData";
+ private static final String EXTENT_KEY = "checkExtent";
+ private static final String SUCCESS = "success";
+
+ @Override
+ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
+ IteratorEnvironment env) throws IOException {
+ Preconditions.checkState(check == null && expectedExtent == null && startKey == null);
+ try {
+ String className = Objects.requireNonNull(options.get(CHECK_CLASS_KEY));
+ String checkData = Objects.requireNonNull(options.get(CHECK_DATA_KEY));
+ log.trace("Instantiating class {} using {}", className, checkData);
+ Class<? extends TabletMetadataCheck> clazz =
+ ClassLoaderUtil.loadClass(null, className, TabletMetadataCheck.class);
+ check = GSON.get().fromJson(checkData, clazz);
+ expectedExtent = KeyExtent.fromBase64(options.get(EXTENT_KEY));
+ this.source = source;
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ @Override
+ public void seek(Range range, Collection<ByteSequence> columnFamilies, boolean inclusive)
+ throws IOException {
+ Preconditions.checkState(check != null && expectedExtent != null);
+
+ this.startKey = null;
+
+ Text tabletRow = getTabletRow(range);
+
+ var expectedMetaRow = expectedExtent.toMetaRow();
+ Preconditions.checkState(tabletRow.equals(expectedMetaRow), "Tablet row mismatch %s %s",
+ tabletRow, expectedMetaRow);
+
+ var colsToRead = check.columnsToRead();
+ // TODO actually use columns in seek and TabletMetadata construction
Review Comment:
Need to remove this TODO and open a follow on issue before merging
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1913416939
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataCheck.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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 java.util.Collections;
+import java.util.EnumSet;
+import java.util.Set;
+
+/**
+ * This interface facilitates atomic checks of tablet metadata prior to updating tablet metadata.
+ * The way it is intended to be used is the following.
+ * <ol>
+ * <li>On the client side a TabletMetadataCheck object is created and passed to Ample</li>
Review Comment:
Yeah I think making sure the impl is tested is the best option for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1913948504
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/TabletMetadataCheckIterator.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.server.metadata.iterators;
+
+import static org.apache.accumulo.core.util.LazySingletons.GSON;
+import static org.apache.accumulo.server.metadata.iterators.ColumnFamilyTransformationIterator.getTabletRow;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.accumulo.core.classloader.ClassLoaderUtil;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.data.ByteSequence;
+import org.apache.accumulo.core.data.Condition;
+import org.apache.accumulo.core.data.Key;
+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.iterators.IteratorAdapter;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletMetadataCheck;
+import org.apache.accumulo.server.metadata.ConditionalTabletMutatorImpl;
+import org.apache.hadoop.io.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Preconditions;
+
+public class TabletMetadataCheckIterator implements SortedKeyValueIterator<Key,Value> {
+
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataCheckIterator.class);
+
+ private TabletMetadataCheck check;
+ private KeyExtent expectedExtent;
+
+ private Key startKey;
+ private Value topValue;
+ private SortedKeyValueIterator<Key,Value> source;
+
+ private static final String CHECK_CLASS_KEY = "checkClass";
+ private static final String CHECK_DATA_KEY = "checkData";
+ private static final String EXTENT_KEY = "checkExtent";
+ private static final String SUCCESS = "success";
+
+ @Override
+ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
+ IteratorEnvironment env) throws IOException {
+ Preconditions.checkState(check == null && expectedExtent == null && startKey == null);
+ try {
+ String className = Objects.requireNonNull(options.get(CHECK_CLASS_KEY));
+ String checkData = Objects.requireNonNull(options.get(CHECK_DATA_KEY));
+ log.trace("Instantiating class {} using {}", className, checkData);
+ Class<? extends TabletMetadataCheck> clazz =
+ ClassLoaderUtil.loadClass(null, className, TabletMetadataCheck.class);
+ check = GSON.get().fromJson(checkData, clazz);
+ expectedExtent = KeyExtent.fromBase64(options.get(EXTENT_KEY));
+ this.source = source;
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ @Override
+ public void seek(Range range, Collection<ByteSequence> columnFamilies, boolean inclusive)
+ throws IOException {
+ Preconditions.checkState(check != null && expectedExtent != null);
+
+ this.startKey = null;
+
+ Text tabletRow = getTabletRow(range);
+
+ var expectedMetaRow = expectedExtent.toMetaRow();
+ Preconditions.checkState(tabletRow.equals(expectedMetaRow), "Tablet row mismatch %s %s",
+ tabletRow, expectedMetaRow);
+
+ var colsToRead = check.columnsToRead();
+ // TODO actually use columns in seek and TabletMetadata construction
Review Comment:
Opened #5254 and removed the TODO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1932566020
##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/queue/ResolvedCompactionJob.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.manager.compaction.queue;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.accumulo.core.client.admin.compaction.CompactableFile;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.fate.FateId;
+import org.apache.accumulo.core.metadata.CompactableFileImpl;
+import org.apache.accumulo.core.metadata.StoredTabletFile;
+import org.apache.accumulo.core.metadata.schema.DataFileValue;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.spi.compaction.CompactionJob;
+import org.apache.accumulo.core.spi.compaction.CompactionKind;
+import org.apache.accumulo.core.spi.compaction.CompactorGroupId;
+
+/**
+ * This class takes a compaction job and the tablet metadata from which is was generated and
+ * computes all information that is needed to actually run the compaction. A goal of this class is
+ * to avoid referencing anything that is not needed. For example only a subset of the information
+ * from TabletMetadata is needed, therefore a reference to the TabletMetadata object should not be
+ * kept. This object will placed on a queue for compaction and that is why it should avoid using
+ * more memory than needed. Before this class was created queued compaction jobs would reference a
+ * tablet metadata object which included all of the tablets files. When tablets had lots of file and
+ * lots compactions against them this could use lots of memory and make the manager unstable at time
+ * when things were already bad.
+ */
+public class ResolvedCompactionJob implements CompactionJob {
+
+ // The fate id of the table compaction that selected files
+ private final FateId selectedFateId;
+ private final Map<StoredTabletFile,DataFileValue> jobFiles;
+ private final CompactionKind kind;
+ private final boolean compactingAll;
+ private final KeyExtent extent;
+ private final short priority;
+ private final CompactorGroupId group;
+ private final String tabletDir;
+ private final boolean overlapsSelectedFiles;
+
+ private static long weigh(Range range) {
+ long estDataSize = 0;
+ if (range != null) {
+ var staryKey = range.getStartKey();
+ estDataSize += staryKey == null ? 0 : staryKey.getSize();
+ var endKey = range.getEndKey();
+ estDataSize += endKey == null ? 0 : endKey.getSize();
+ }
+ return estDataSize;
+ }
+
+ public static final SizeTrackingTreeMap.Weigher<CompactionJob> WEIGHER = job -> {
+ if (job instanceof ResolvedCompactionJob) {
+ var rcj = (ResolvedCompactionJob) job;
+ long estDataSize = 0;
+ if (rcj.selectedFateId != null) {
+ estDataSize += rcj.selectedFateId.canonical().length();
+ }
+ for (var file : rcj.jobFiles.keySet()) {
+ estDataSize += file.getMetadataPath().length();
+ estDataSize += 24; // There are three longs in DataFileValue
+ estDataSize += weigh(file.getRange());
+ }
+
+ estDataSize += rcj.group.canonical().length();
+ estDataSize += rcj.extent.tableId().canonical().length();
+ estDataSize += rcj.extent.prevEndRow() == null ? 0 : rcj.extent.prevEndRow().getLength();
+ estDataSize += rcj.extent.endRow() == null ? 0 : rcj.extent.endRow().getLength();
+ estDataSize += rcj.tabletDir.length();
+ // Guess at how many bytes the overhead at things like pointers and primitives are taking in
+ // this object.
+ estDataSize += 64;
+ return estDataSize;
+ } else {
+ // Do not know the concrete type so weigh based on the interface methods of CompaactionJob
Review Comment:
I missed this comment when merging. Pushed aa1f30aa64fd45ff0844fe1e13b749c85643472e to main to pull in this suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1912239650
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/TabletMetadataCheckIterator.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.server.metadata.iterators;
+
+import static org.apache.accumulo.core.util.LazySingletons.GSON;
+import static org.apache.accumulo.server.metadata.iterators.ColumnFamilyTransformationIterator.getTabletRow;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.accumulo.core.classloader.ClassLoaderUtil;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.data.ByteSequence;
+import org.apache.accumulo.core.data.Condition;
+import org.apache.accumulo.core.data.Key;
+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.iterators.IteratorAdapter;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletMetadataCheck;
+import org.apache.accumulo.server.metadata.ConditionalTabletMutatorImpl;
+import org.apache.hadoop.io.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Preconditions;
+
+public class TabletMetadataCheckIterator implements SortedKeyValueIterator<Key,Value> {
+
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataCheckIterator.class);
+
+ private TabletMetadataCheck check;
+ private KeyExtent expectedExtent;
+
+ private Key startKey;
+ private Value topValue;
+ private SortedKeyValueIterator<Key,Value> source;
+
+ private static final String CHECK_CLASS_KEY = "checkClass";
+ private static final String CHECK_DATA_KEY = "checkData";
+ private static final String EXTENT_KEY = "checkExtent";
+ private static final String SUCCESS = "success";
+
+ @Override
+ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
+ IteratorEnvironment env) throws IOException {
+ Preconditions.checkState(check == null && expectedExtent == null && startKey == null);
+ try {
+ String className = Objects.requireNonNull(options.get(CHECK_CLASS_KEY));
+ String checkData = Objects.requireNonNull(options.get(CHECK_DATA_KEY));
+ log.trace("Instantiating class {} using {}", className, checkData);
+ Class<? extends TabletMetadataCheck> clazz =
+ ClassLoaderUtil.loadClass(null, className, TabletMetadataCheck.class);
+ check = GSON.get().fromJson(checkData, clazz);
+ expectedExtent = KeyExtent.fromBase64(options.get(EXTENT_KEY));
+ this.source = source;
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
Review Comment:
Would it be useful to add any logging here for a ClassNotFoundException if the impl isn't on the server classpath? It may not be necessary if this exception will be logged higher up when the IllegalStateException is thrown.
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/TabletMetadataCheckIterator.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.server.metadata.iterators;
+
+import static org.apache.accumulo.core.util.LazySingletons.GSON;
+import static org.apache.accumulo.server.metadata.iterators.ColumnFamilyTransformationIterator.getTabletRow;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.accumulo.core.classloader.ClassLoaderUtil;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.data.ByteSequence;
+import org.apache.accumulo.core.data.Condition;
+import org.apache.accumulo.core.data.Key;
+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.iterators.IteratorAdapter;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletMetadataCheck;
+import org.apache.accumulo.server.metadata.ConditionalTabletMutatorImpl;
+import org.apache.hadoop.io.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Preconditions;
+
+public class TabletMetadataCheckIterator implements SortedKeyValueIterator<Key,Value> {
+
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataCheckIterator.class);
+
+ private TabletMetadataCheck check;
+ private KeyExtent expectedExtent;
+
+ private Key startKey;
+ private Value topValue;
+ private SortedKeyValueIterator<Key,Value> source;
+
+ private static final String CHECK_CLASS_KEY = "checkClass";
+ private static final String CHECK_DATA_KEY = "checkData";
+ private static final String EXTENT_KEY = "checkExtent";
+ private static final String SUCCESS = "success";
+
+ @Override
+ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
+ IteratorEnvironment env) throws IOException {
+ Preconditions.checkState(check == null && expectedExtent == null && startKey == null);
+ try {
+ String className = Objects.requireNonNull(options.get(CHECK_CLASS_KEY));
+ String checkData = Objects.requireNonNull(options.get(CHECK_DATA_KEY));
+ log.trace("Instantiating class {} using {}", className, checkData);
+ Class<? extends TabletMetadataCheck> clazz =
+ ClassLoaderUtil.loadClass(null, className, TabletMetadataCheck.class);
+ check = GSON.get().fromJson(checkData, clazz);
+ expectedExtent = KeyExtent.fromBase64(options.get(EXTENT_KEY));
+ this.source = source;
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ @Override
+ public void seek(Range range, Collection<ByteSequence> columnFamilies, boolean inclusive)
+ throws IOException {
+ Preconditions.checkState(check != null && expectedExtent != null);
+
+ this.startKey = null;
+
+ Text tabletRow = getTabletRow(range);
+
+ var expectedMetaRow = expectedExtent.toMetaRow();
+ Preconditions.checkState(tabletRow.equals(expectedMetaRow), "Tablet row mismatch %s %s",
+ tabletRow, expectedMetaRow);
+
+ var colsToRead = check.columnsToRead();
+ // TODO actually use columns in seek and TabletMetadata construction
Review Comment:
I was actually just getting ready to ask you if you planned to do this now or in a future PR
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadataCheck.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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 java.util.Collections;
+import java.util.EnumSet;
+import java.util.Set;
+
+/**
+ * This interface facilitates atomic checks of tablet metadata prior to updating tablet metadata.
+ * The way it is intended to be used is the following.
+ * <ol>
+ * <li>On the client side a TabletMetadataCheck object is created and passed to Ample</li>
Review Comment:
I'm wondering if there is any good way to specifically validate that an implementation can be correctly serialized by gson, especially because we've configured our gson to not allow unsafe usage (so we require a default constructor and not using final etc). I'm guessing the only way is just to to make sure there are tests that use the new check in a conditional mutation and verify it works which is probably fine as we should be writing tests anyways.
##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -575,97 +496,50 @@ private ReserveCompactionTask(CompactionJobQueues.MetaJob metaJob, String compac
@Override
public CompactionMetadata get() {
- try {
- var metaJob = metaJobRef.get();
- if (metaJob == null) {
- LOG.warn("Compaction reservation request for {} {} was garbage collected.",
- compactorAddress, externalCompactionId);
- return null;
- }
-
- var tabletMetadata = metaJob.getTabletMetadata();
-
- var jobFiles = metaJob.getJob().getFiles().stream()
- .map(CompactableFileImpl::toStoredTabletFile).collect(Collectors.toSet());
-
- Retry retry = Retry.builder().maxRetries(5).retryAfter(Duration.ofMillis(100))
- .incrementBy(Duration.ofMillis(100)).maxWait(Duration.ofSeconds(10)).backOffFactor(1.5)
- .logInterval(Duration.ofMinutes(3)).createRetry();
-
- while (retry.canRetry()) {
- try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
- var extent = metaJob.getTabletMetadata().getExtent();
-
- if (!canReserveCompaction(tabletMetadata, metaJob.getJob().getKind(), jobFiles, ctx,
- manager.getSteadyTime())) {
- return null;
- }
-
- var ecm = createExternalCompactionMetadata(metaJob.getJob(), jobFiles, tabletMetadata,
- compactorAddress, externalCompactionId);
-
- // any data that is read from the tablet to make a decision about if it can compact or
- // not
- // must be checked for changes in the conditional mutation.
- var tabletMutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation()
- .requireFiles(jobFiles).requireNotCompacting(jobFiles);
- if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
- // For system compactions the user compaction requested column is examined when
- // deciding
- // if a compaction can start so need to check for changes to this column.
- tabletMutator.requireSame(tabletMetadata, SELECTED, USER_COMPACTION_REQUESTED);
- } else {
- tabletMutator.requireSame(tabletMetadata, SELECTED);
- }
-
- if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
- var selectedFiles = tabletMetadata.getSelectedFiles();
- var reserved =
- getFilesReservedBySelection(tabletMetadata, manager.getSteadyTime(), ctx);
-
- // If there is a selectedFiles column, and the reserved set is empty this means that
- // either no user jobs were completed yet or the selection expiration time has passed
- // so the column is eligible to be deleted so a system job can run instead
- if (selectedFiles != null && reserved.isEmpty()
- && !Collections.disjoint(jobFiles, selectedFiles.getFiles())) {
- LOG.debug("Deleting user compaction selected files for {} {}", extent,
- externalCompactionId);
- tabletMutator.deleteSelectedFiles();
- }
- }
+ if (ctx.getTableState(rcJob.getExtent().tableId()) != TableState.ONLINE) {
+ return null;
+ }
- tabletMutator.putExternalCompaction(externalCompactionId, ecm);
- tabletMutator.submit(
- tm -> tm.getExternalCompactions().containsKey(externalCompactionId),
- () -> "compaction reservation");
+ try {
+ try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
Review Comment:
the changes here are really nice, it's definitely a lot simpler. I think it would make moving the reservation to the Compactor easier as well if we implement #4978
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1912241304
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/TabletMetadataCheckIterator.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.server.metadata.iterators;
+
+import static org.apache.accumulo.core.util.LazySingletons.GSON;
+import static org.apache.accumulo.server.metadata.iterators.ColumnFamilyTransformationIterator.getTabletRow;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.accumulo.core.classloader.ClassLoaderUtil;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.data.ByteSequence;
+import org.apache.accumulo.core.data.Condition;
+import org.apache.accumulo.core.data.Key;
+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.iterators.IteratorAdapter;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletMetadataCheck;
+import org.apache.accumulo.server.metadata.ConditionalTabletMutatorImpl;
+import org.apache.hadoop.io.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Preconditions;
+
+public class TabletMetadataCheckIterator implements SortedKeyValueIterator<Key,Value> {
+
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataCheckIterator.class);
+
+ private TabletMetadataCheck check;
+ private KeyExtent expectedExtent;
+
+ private Key startKey;
+ private Value topValue;
+ private SortedKeyValueIterator<Key,Value> source;
+
+ private static final String CHECK_CLASS_KEY = "checkClass";
+ private static final String CHECK_DATA_KEY = "checkData";
+ private static final String EXTENT_KEY = "checkExtent";
+ private static final String SUCCESS = "success";
+
+ @Override
+ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
+ IteratorEnvironment env) throws IOException {
+ Preconditions.checkState(check == null && expectedExtent == null && startKey == null);
+ try {
+ String className = Objects.requireNonNull(options.get(CHECK_CLASS_KEY));
+ String checkData = Objects.requireNonNull(options.get(CHECK_DATA_KEY));
+ log.trace("Instantiating class {} using {}", className, checkData);
+ Class<? extends TabletMetadataCheck> clazz =
+ ClassLoaderUtil.loadClass(null, className, TabletMetadataCheck.class);
+ check = GSON.get().fromJson(checkData, clazz);
+ expectedExtent = KeyExtent.fromBase64(options.get(EXTENT_KEY));
+ this.source = source;
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ @Override
+ public void seek(Range range, Collection<ByteSequence> columnFamilies, boolean inclusive)
+ throws IOException {
+ Preconditions.checkState(check != null && expectedExtent != null);
+
+ this.startKey = null;
+
+ Text tabletRow = getTabletRow(range);
+
+ var expectedMetaRow = expectedExtent.toMetaRow();
+ Preconditions.checkState(tabletRow.equals(expectedMetaRow), "Tablet row mismatch %s %s",
+ tabletRow, expectedMetaRow);
+
+ var colsToRead = check.columnsToRead();
+ // TODO actually use columns in seek and TabletMetadata construction
Review Comment:
> Need to remove this TODO and open a follow on issue before merging
I was actually just getting ready to ask you if you planned to do this now or in a future PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1929374177
##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/queue/ResolvedCompactionJob.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * 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.manager.compaction.queue;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.accumulo.core.client.admin.compaction.CompactableFile;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.fate.FateId;
+import org.apache.accumulo.core.metadata.CompactableFileImpl;
+import org.apache.accumulo.core.metadata.StoredTabletFile;
+import org.apache.accumulo.core.metadata.schema.DataFileValue;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.spi.compaction.CompactionJob;
+import org.apache.accumulo.core.spi.compaction.CompactionKind;
+import org.apache.accumulo.core.spi.compaction.CompactorGroupId;
+
+/**
+ * This class takes a compaction job and the tablet metadata from which is was generated and
+ * computes all information that is needed to actually run the compaction. A goal of this class is
+ * to avoid referencing anything that is not needed. For example only a subset of the information
+ * from TabletMetadata is needed, therefore a reference to the TabletMetadata object should not be
+ * kept. This object will placed on a queue for compaction and that is why it should avoid using
+ * more memory than needed. Before this class was created queued compaction jobs would reference a
+ * tablet metadata object which included all of the tablets files. When tablets had lots of file and
+ * lots compactions against them this could use lots of memory and make the manager unstable at time
+ * when things were already bad.
+ */
+public class ResolvedCompactionJob implements CompactionJob {
+
+ // The fate id of the table compaction that selected files
+ private final FateId selectedFateId;
+ private final Map<StoredTabletFile,DataFileValue> jobFiles;
+ private final CompactionKind kind;
+ private final boolean compactingAll;
+ private final KeyExtent extent;
+ private final short priority;
+ private final CompactorGroupId group;
+ private final String tabletDir;
+ private final boolean overlapsSelectedFiles;
+
+ private static long weigh(Range range) {
+ long estDataSize = 0;
+ if (range != null) {
+ var staryKey = range.getStartKey();
+ estDataSize += staryKey == null ? 0 : staryKey.getSize();
+ var endKey = range.getEndKey();
+ estDataSize += endKey == null ? 0 : endKey.getSize();
+ }
+ return estDataSize;
+ }
+
+ public static final SizeTrackingTreeMap.Weigher<CompactionJob> WEIGHER = job -> {
+ if (job instanceof ResolvedCompactionJob) {
+ var rcj = (ResolvedCompactionJob) job;
+ long estDataSize = 0;
+ if (rcj.selectedFateId != null) {
+ estDataSize += rcj.selectedFateId.canonical().length();
+ }
+ for (var file : rcj.jobFiles.keySet()) {
+ estDataSize += file.getMetadataPath().length();
+ estDataSize += 24; // There are three longs in DataFileValue
+ estDataSize += weigh(file.getRange());
+ }
+
+ estDataSize += rcj.group.canonical().length();
+ estDataSize += rcj.extent.tableId().canonical().length();
+ estDataSize += rcj.extent.prevEndRow() == null ? 0 : rcj.extent.prevEndRow().getLength();
+ estDataSize += rcj.extent.endRow() == null ? 0 : rcj.extent.endRow().getLength();
+ estDataSize += rcj.tabletDir.length();
+ // Guess at how many bytes the overhead at things like pointers and primitives are taking in
+ // this object.
+ estDataSize += 64;
+ return estDataSize;
+ } else {
+ // Do not know the concrete type so weigh based on the interface methods of CompaactionJob
Review Comment:
```suggestion
// Do not know the concrete type so weigh based on the interface methods of CompactionJob
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#discussion_r1913415773
##########
server/base/src/main/java/org/apache/accumulo/server/metadata/iterators/TabletMetadataCheckIterator.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.server.metadata.iterators;
+
+import static org.apache.accumulo.core.util.LazySingletons.GSON;
+import static org.apache.accumulo.server.metadata.iterators.ColumnFamilyTransformationIterator.getTabletRow;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.accumulo.core.classloader.ClassLoaderUtil;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.data.ByteSequence;
+import org.apache.accumulo.core.data.Condition;
+import org.apache.accumulo.core.data.Key;
+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.iterators.IteratorAdapter;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletMetadataCheck;
+import org.apache.accumulo.server.metadata.ConditionalTabletMutatorImpl;
+import org.apache.hadoop.io.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Preconditions;
+
+public class TabletMetadataCheckIterator implements SortedKeyValueIterator<Key,Value> {
+
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataCheckIterator.class);
+
+ private TabletMetadataCheck check;
+ private KeyExtent expectedExtent;
+
+ private Key startKey;
+ private Value topValue;
+ private SortedKeyValueIterator<Key,Value> source;
+
+ private static final String CHECK_CLASS_KEY = "checkClass";
+ private static final String CHECK_DATA_KEY = "checkData";
+ private static final String EXTENT_KEY = "checkExtent";
+ private static final String SUCCESS = "success";
+
+ @Override
+ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
+ IteratorEnvironment env) throws IOException {
+ Preconditions.checkState(check == null && expectedExtent == null && startKey == null);
+ try {
+ String className = Objects.requireNonNull(options.get(CHECK_CLASS_KEY));
+ String checkData = Objects.requireNonNull(options.get(CHECK_DATA_KEY));
+ log.trace("Instantiating class {} using {}", className, checkData);
+ Class<? extends TabletMetadataCheck> clazz =
+ ClassLoaderUtil.loadClass(null, className, TabletMetadataCheck.class);
+ check = GSON.get().fromJson(checkData, clazz);
+ expectedExtent = KeyExtent.fromBase64(options.get(EXTENT_KEY));
+ this.source = source;
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
Review Comment:
It depends on if the existing exception messages include the class name. Could add the class name to the IllegalStateException to be sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #5247:
URL: https://github.com/apache/accumulo/pull/5247
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Avoids holding tablet metadata for queued compactions [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on PR #5247:
URL: https://github.com/apache/accumulo/pull/5247#issuecomment-2599032719
Merged in the changes from main which include #5252. Wrote a more efficient compaction job weigher that works with the new ResolvedCompactionJob in this PR. The initial weigher in #5252 was pretty inefficient, but I knew it would need to change in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org