This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push:
new f4a857e1fa moves static metadata util methods to only places using
them (#4639)
f4a857e1fa is described below
commit f4a857e1fa46d5de684e81aa5dd4a8a96e5097b2
Author: Keith Turner <[email protected]>
AuthorDate: Wed Jun 5 17:25:40 2024 -0400
moves static metadata util methods to only places using them (#4639)
While investigating #3858, noticed these utility metadata utility
methods that were not widely used. Moved the static method where
they are actually used to clean up the code.
---
.../accumulo/server/util/MetadataTableUtil.java | 53 ----------------------
.../manager/upgrade/SplitRecovery12to13.java | 42 +++++++++++++++--
.../accumulo/test/MetaConstraintRetryIT.java | 4 +-
.../accumulo/test/functional/SplitRecoveryIT.java | 18 +++++++-
4 files changed, 58 insertions(+), 59 deletions(-)
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
index e767bca510..4e38572dd3 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
@@ -31,11 +31,9 @@ import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType
import java.io.IOException;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import java.util.Map.Entry;
import java.util.SortedMap;
import java.util.TreeMap;
@@ -57,14 +55,11 @@ 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;
-import org.apache.accumulo.core.fate.FateId;
import org.apache.accumulo.core.gc.ReferenceFile;
import org.apache.accumulo.core.lock.ServiceLock;
import org.apache.accumulo.core.metadata.AccumuloTable;
-import org.apache.accumulo.core.metadata.ReferencedTabletFile;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.schema.Ample;
-import org.apache.accumulo.core.metadata.schema.Ample.TabletMutator;
import org.apache.accumulo.core.metadata.schema.DataFileValue;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ClonedColumnFamily;
@@ -72,7 +67,6 @@ import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Cu
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily;
-import org.apache.accumulo.core.metadata.schema.MetadataTime;
import org.apache.accumulo.core.metadata.schema.TabletDeletedException;
import org.apache.accumulo.core.metadata.schema.TabletMetadata;
import org.apache.accumulo.core.metadata.schema.TabletsMetadata;
@@ -103,53 +97,6 @@ public class MetadataTableUtil {
new Value(zooLock.getLockID().serialize(context.getZooKeeperRoot() +
"/")));
}
- public static void update(ServerContext context, ServiceLock zooLock,
Mutation m,
- KeyExtent extent) {
-
- if (zooLock != null) {
- putLockID(context, zooLock, m);
- }
-
- String metaTable = Ample.DataLevel.of(extent.tableId()).metaTable();
- while (true) {
- try (BatchWriter writer = context.createBatchWriter(metaTable)) {
- writer.addMutation(m);
- writer.flush();
- return;
- } catch (MutationsRejectedException e) {
-
- if (!e.getConstraintViolationSummaries().isEmpty()) {
- // retrying when a CVE occurs is probably futile and can cause
problems, see ACCUMULO-3096
- throw new IllegalArgumentException(e);
- }
- } catch (TableNotFoundException e) {
- logUpdateFailure(m, extent, e);
- }
-
- sleepUninterruptibly(1, TimeUnit.SECONDS);
- }
- }
-
- private static void logUpdateFailure(Mutation m, KeyExtent extent, Exception
e) {
- log.error("Failed to write metadata updates for extent {} {}", extent,
m.prettyPrint(), e);
- }
-
- public static Map<StoredTabletFile,DataFileValue>
updateTabletDataFile(FateId fateId,
- KeyExtent extent, Map<ReferencedTabletFile,DataFileValue> estSizes,
MetadataTime time,
- ServerContext context, ServiceLock zooLock) {
- TabletMutator tablet = context.getAmple().mutateTablet(extent);
- tablet.putTime(time);
-
- Map<StoredTabletFile,DataFileValue> newFiles = new
HashMap<>(estSizes.size());
- estSizes.forEach((tf, dfv) -> {
- tablet.putFile(tf, dfv);
- tablet.putBulkFile(tf, fateId);
- newFiles.put(tf.insert(), dfv);
- });
- tablet.mutate();
- return newFiles;
- }
-
public static void deleteTable(TableId tableId, boolean insertDeletes,
ServerContext context,
ServiceLock lock) throws AccumuloException {
try (
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/SplitRecovery12to13.java
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/SplitRecovery12to13.java
index 436f96c5d4..73d2b66bdf 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/SplitRecovery12to13.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/SplitRecovery12to13.java
@@ -18,6 +18,7 @@
*/
package org.apache.accumulo.manager.upgrade;
+import static
com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
import static
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Upgrade12to13.OLD_PREV_ROW_COLUMN;
import static
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Upgrade12to13.SPLIT_RATIO_COLUMN;
@@ -29,9 +30,13 @@ import java.util.Map.Entry;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
+import java.util.concurrent.TimeUnit;
import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.MutationsRejectedException;
import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.accumulo.core.clientImpl.ScannerImpl;
import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.Mutation;
@@ -40,6 +45,7 @@ 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;
+import org.apache.accumulo.core.lock.ServiceLock;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.schema.Ample;
import org.apache.accumulo.core.metadata.schema.DataFileValue;
@@ -203,7 +209,7 @@ public class SplitRecovery12to13 {
Mutation m = TabletsSection.TabletColumnFamily.createPrevRowMutation(ke);
SPLIT_RATIO_COLUMN.putDelete(m);
OLD_PREV_ROW_COLUMN.putDelete(m);
- MetadataTableUtil.update(context, null, m,
KeyExtent.fromMetaRow(metadataEntry));
+ update(context, null, m, KeyExtent.fromMetaRow(metadataEntry));
}
public static void splitTablet(KeyExtent extent, Text oldPrevEndRow, double
splitRatio,
@@ -217,7 +223,7 @@ public class SplitRecovery12to13 {
ecids.forEach(ecid ->
m.putDelete(TabletsSection.ExternalCompactionColumnFamily.STR_NAME,
ecid.canonical()));
- MetadataTableUtil.update(context, null, m, extent);
+ update(context, null, m, extent);
}
public static void finishSplit(Text metadataEntry,
@@ -236,7 +242,7 @@ public class SplitRecovery12to13 {
m.putDelete(DataFileColumnFamily.NAME, pathToRemove.getMetadataText());
}
- MetadataTableUtil.update(context, null, m,
KeyExtent.fromMetaRow(metadataEntry));
+ update(context, null, m, KeyExtent.fromMetaRow(metadataEntry));
}
public static void finishSplit(KeyExtent extent,
@@ -245,4 +251,34 @@ public class SplitRecovery12to13 {
finishSplit(extent.toMetaRow(), datafileSizes, highDatafilesToRemove,
context);
}
+ public static void update(ServerContext context, ServiceLock zooLock,
Mutation m,
+ KeyExtent extent) {
+
+ if (zooLock != null) {
+ MetadataTableUtil.putLockID(context, zooLock, m);
+ }
+
+ String metaTable = Ample.DataLevel.of(extent.tableId()).metaTable();
+ while (true) {
+ try (BatchWriter writer = context.createBatchWriter(metaTable)) {
+ writer.addMutation(m);
+ writer.flush();
+ return;
+ } catch (MutationsRejectedException e) {
+
+ if (!e.getConstraintViolationSummaries().isEmpty()) {
+ // retrying when a CVE occurs is probably futile and can cause
problems, see ACCUMULO-3096
+ throw new IllegalArgumentException(e);
+ }
+ } catch (TableNotFoundException e) {
+ logUpdateFailure(m, extent, e);
+ }
+
+ sleepUninterruptibly(1, TimeUnit.SECONDS);
+ }
+ }
+
+ private static void logUpdateFailure(Mutation m, KeyExtent extent, Exception
e) {
+ log.error("Failed to write metadata updates for extent {} {}", extent,
m.prettyPrint(), e);
+ }
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/MetaConstraintRetryIT.java
b/test/src/main/java/org/apache/accumulo/test/MetaConstraintRetryIT.java
index 2bf338bde9..6d08a6af49 100644
--- a/test/src/main/java/org/apache/accumulo/test/MetaConstraintRetryIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/MetaConstraintRetryIT.java
@@ -33,8 +33,8 @@ import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.core.metadata.AccumuloTable;
import org.apache.accumulo.core.security.TablePermission;
import org.apache.accumulo.harness.AccumuloClusterHarness;
+import org.apache.accumulo.manager.upgrade.SplitRecovery12to13;
import org.apache.accumulo.server.ServerContext;
-import org.apache.accumulo.server.util.MetadataTableUtil;
import org.junit.jupiter.api.Test;
public class MetaConstraintRetryIT extends AccumuloClusterHarness {
@@ -58,7 +58,7 @@ public class MetaConstraintRetryIT extends
AccumuloClusterHarness {
// unknown columns should cause constraint violation
m.put("badcolfam", "badcolqual", "3");
var iae = assertThrows(IllegalArgumentException.class,
- () -> MetadataTableUtil.update(context, null, m, extent));
+ () -> SplitRecovery12to13.update(context, null, m, extent));
assertEquals(MutationsRejectedException.class,
iae.getCause().getClass());
var mre = (MutationsRejectedException) iae.getCause();
assertFalse(mre.getConstraintViolationSummaries().isEmpty());
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java
index 524464d9fa..88c3b2b894 100644
---
a/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java
@@ -74,6 +74,22 @@ import org.junit.jupiter.api.Test;
public class SplitRecoveryIT extends ConfigurableMacBase {
+ public static Map<StoredTabletFile,DataFileValue>
updateTabletDataFile(FateId fateId,
+ KeyExtent extent, Map<ReferencedTabletFile,DataFileValue> estSizes,
MetadataTime time,
+ ServerContext context, ServiceLock zooLock) {
+ TabletMutator tablet = context.getAmple().mutateTablet(extent);
+ tablet.putTime(time);
+
+ Map<StoredTabletFile,DataFileValue> newFiles = new
HashMap<>(estSizes.size());
+ estSizes.forEach((tf, dfv) -> {
+ tablet.putFile(tf, dfv);
+ tablet.putBulkFile(tf, fateId);
+ newFiles.put(tf.insert(), dfv);
+ });
+ tablet.mutate();
+ return newFiles;
+ }
+
@Override
protected Duration defaultTimeout() {
return Duration.ofMinutes(1);
@@ -140,7 +156,7 @@ public class SplitRecoveryIT extends ConfigurableMacBase {
FateId fateId =
FateId.from(FateInstanceType.fromTableId(extent.tableId()),
UUID.randomUUID());
SortedMap<StoredTabletFile,DataFileValue> storedFiles =
- new TreeMap<>(MetadataTableUtil.updateTabletDataFile(fateId, extent,
dataFiles,
+ new TreeMap<>(updateTabletDataFile(fateId, extent, dataFiles,
new MetadataTime(0, TimeType.LOGICAL), context, zl));
if (i == extentToSplit) {
splitDataFiles = storedFiles;
