This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new a47ee0b77d Polish up some code from recent changes (#3979)
a47ee0b77d is described below
commit a47ee0b77da47e9115caad28f18a1f190dd30c2e
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Wed Nov 29 03:03:29 2023 -0500
Polish up some code from recent changes (#3979)
* Use simpler regex pattern for non-digits
* Minimize deprecated "chopped" column code
* Remove unused logger
* Remove unnecessary 'var ignored = ' which caused unused variable
warnings in some IDEs; this might cause warnings about unused return
values in other IDEs, but I'm not sure the ideal way to address that
at this point; for now, this change keeps us at 0 warnings in Eclipse
with default settings, except those from thrift and the unavoidable
ones from Eclipse's incorrect handling of forRemoval deprecations, and
this is also consistent with our current coding style (we don't use
ignored variable declarations anywhere else)
* Replace some static imports to inner classes with non-static imports,
as this results in the same code usage; static is unnecessary there
* Fix assertion in DefaultCompactionPlannerTest found by unused variable
warning
* Make slight improvements to upgrader code to identify and retrieve the
oldest upgradable version
---
.../apache/accumulo/core/conf/PropertyType.java | 2 +-
.../core/metadata/schema/MetadataSchema.java | 10 ++++++
.../core/metadata/schema/RootTabletMetadata.java | 3 --
.../schema/UpgraderDeprecatedConstants.java | 40 ----------------------
.../accumulo/core/tabletserver/log/LogEntry.java | 4 +--
.../compaction/DefaultCompactionPlannerTest.java | 6 ++--
.../accumulo/server/AccumuloDataVersion.java | 12 ++++---
.../server/constraints/MetadataConstraints.java | 7 ++--
.../apache/accumulo/server/ServerContextTest.java | 7 ++--
.../accumulo/manager/upgrade/Upgrader11to12.java | 14 ++++----
.../manager/upgrade/Upgrader11to12Test.java | 7 ++--
11 files changed, 44 insertions(+), 68 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
index 8696ae295f..e503b92c36 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
@@ -245,7 +245,7 @@ public enum PropertyType {
|| (suffixCheck.test(x) && new Bounds(lowerBound,
upperBound).test(stripUnits.apply(x)));
}
- private static final Pattern SUFFIX_REGEX = Pattern.compile("[^\\d]*$");
+ private static final Pattern SUFFIX_REGEX = Pattern.compile("\\D*$"); //
match non-digits at end
private static final Function<String,String> stripUnits =
x -> x == null ? null : SUFFIX_REGEX.matcher(x.trim()).replaceAll("");
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 2bc0a6e18e..0fd991296c 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
@@ -321,6 +321,16 @@ public class MetadataSchema {
public static final Text NAME = new Text(STR_NAME);
}
+ /**
+ * Column family for indicating that the files in a tablet have been
trimmed to only include
+ * data for the current tablet, so that they are safe to merge
+ */
+ public static class ChoppedColumnFamily {
+ // kept to support upgrades to 3.1; name is used for both col fam and
col qual
+ @Deprecated(since = "3.1.0")
+ public static final Text NAME = new Text("chopped");
+ }
+
public static class ExternalCompactionColumnFamily {
public static final String STR_NAME = "ecomp";
public static final Text NAME = new Text(STR_NAME);
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
index 19285bebdb..cc16bb6ff9 100644
---
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
+++
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java
@@ -39,8 +39,6 @@ import org.apache.accumulo.core.metadata.RootTable;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.CurrentLocationColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.FutureLocationColumnFamily;
import org.apache.hadoop.io.Text;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* This class is used to serialize and deserialize root tablet metadata using
GSon. The only data
@@ -50,7 +48,6 @@ import org.slf4j.LoggerFactory;
*/
public class RootTabletMetadata {
- private static final Logger log =
LoggerFactory.getLogger(RootTabletMetadata.class);
private static final CharsetDecoder UTF8_error_detecting_decoder =
UTF_8.newDecoder();
private static final Predicate<Entry<String,TreeMap<String,String>>>
isLocationCF = e -> {
String fam = e.getKey();
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UpgraderDeprecatedConstants.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/UpgraderDeprecatedConstants.java
deleted file mode 100644
index 1d9967cc71..0000000000
---
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/UpgraderDeprecatedConstants.java
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * 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 org.apache.accumulo.core.util.ColumnFQ;
-import org.apache.hadoop.io.Text;
-
-/**
- * MetadataSchema constants that are deprecated and should only be used to
support removals during
- * the upgrade process.
- */
-public class UpgraderDeprecatedConstants {
-
- /**
- * ChoppedColumnFamily kept around for cleaning up old entries on upgrade.
Currently not used,
- * will be used by #3768
- */
- public static class ChoppedColumnFamily {
- public static final String STR_NAME = "chopped";
- public static final Text NAME = new Text(STR_NAME);
- public static final ColumnFQ CHOPPED_COLUMN = new ColumnFQ(NAME, new
Text(STR_NAME));
- }
-
-}
diff --git
a/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
b/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
index d3b6f5ac02..9c592b2260 100644
--- a/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
+++ b/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
@@ -61,7 +61,7 @@ public class LogEntry {
String uuidPart = parts[parts.length - 1];
try {
- var ignored = HostAndPort.fromString(tserverPart);
+ HostAndPort.fromString(tserverPart);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
"Invalid tserver format in filePath. Expected format: host:port.
Found '" + tserverPart
@@ -69,7 +69,7 @@ public class LogEntry {
}
try {
- var ignored = UUID.fromString(uuidPart);
+ UUID.fromString(uuidPart);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Expected valid UUID. Found '" +
uuidPart + "'");
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java
b/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java
index 5ae90a4f2d..c043eac841 100644
---
a/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java
@@ -19,7 +19,6 @@
package org.apache.accumulo.core.spi.compaction;
import static com.google.common.collect.MoreCollectors.onlyElement;
-import static
org.apache.accumulo.core.spi.compaction.CompactionPlanner.InitParameters;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -41,6 +40,7 @@ import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.spi.common.ServiceEnvironment;
import org.apache.accumulo.core.spi.common.ServiceEnvironment.Configuration;
import org.apache.accumulo.core.spi.compaction.CompactionPlan.Builder;
+import
org.apache.accumulo.core.spi.compaction.CompactionPlanner.InitParameters;
import org.apache.accumulo.core.util.compaction.CompactionExecutorIdImpl;
import org.apache.accumulo.core.util.compaction.CompactionPlanImpl;
import org.easymock.EasyMock;
@@ -421,9 +421,9 @@ public class DefaultCompactionPlannerTest {
var params = getInitParamQueues(senv, "");
assertNotNull(params);
- var err = assertThrows(IllegalStateException.class, () ->
planner.init(params),
+ var e2 = assertThrows(IllegalStateException.class, () ->
planner.init(params),
"Failed to throw error");
- assertEquals("No defined executors or queues for this planner",
e.getMessage(),
+ assertEquals("No defined executors or queues for this planner",
e2.getMessage(),
"Error message was not equal");
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/AccumuloDataVersion.java
b/server/base/src/main/java/org/apache/accumulo/server/AccumuloDataVersion.java
index 4aae44a974..5cbb0e2427 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/AccumuloDataVersion.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/AccumuloDataVersion.java
@@ -38,7 +38,7 @@ public class AccumuloDataVersion {
/**
* version (12) reflect changes to support no chop merges including json
encoding of the file
- * column family stored in root and metadata tables.
+ * column family stored in root and metadata tables in version 3.1
*/
public static final int METADATA_FILE_JSON_ENCODING = 12;
@@ -49,12 +49,12 @@ public class AccumuloDataVersion {
/**
* version (10) reflects changes to how root tablet metadata is serialized
in zookeeper starting
- * with 2.1. See {@link
org.apache.accumulo.core.metadata.schema.RootTabletMetadata}.
+ * with 2.1. See {@link
org.apache.accumulo.core.metadata.schema.RootTabletMetadata}
*/
public static final int ROOT_TABLET_META_CHANGES = 10;
/**
- * Historic data versions
+ * Historic data versions.
*
* <ul>
* <li>version (9) RFiles and wal crypto serialization changes. RFile
summary data in 2.0.0</li>
@@ -93,8 +93,12 @@ public class AccumuloDataVersion {
return cv;
}
+ public static int oldestUpgradeableVersion() {
+ return CAN_RUN.stream().mapToInt(x -> x).min().orElseThrow();
+ }
+
public static String oldestUpgradeableVersionName() {
- return dataVersionToReleaseName(CAN_RUN.stream().mapToInt(x ->
x).min().orElseThrow());
+ return dataVersionToReleaseName(oldestUpgradeableVersion());
}
private static String dataVersionToReleaseName(final int version) {
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 ca45a05e8a..aa9772a159 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
@@ -40,6 +40,7 @@ import org.apache.accumulo.core.metadata.MetadataTable;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.schema.DataFileValue;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.BulkFileColumnFamily;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ChoppedColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ClonedColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.CurrentLocationColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
@@ -52,7 +53,6 @@ import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Sc
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily;
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.UpgraderDeprecatedConstants;
import org.apache.accumulo.core.util.ColumnFQ;
import org.apache.accumulo.core.util.cleaner.CleanerUtil;
import org.apache.accumulo.server.ServerContext;
@@ -89,6 +89,9 @@ public class MetadataConstraints implements Constraint {
ServerColumnFamily.FLUSH_COLUMN,
ServerColumnFamily.COMPACT_COLUMN);
+ @SuppressWarnings("deprecation")
+ private static final Text CHOPPED = ChoppedColumnFamily.NAME;
+
private static final Set<Text> validColumnFams =
Set.of(BulkFileColumnFamily.NAME,
LogColumnFamily.NAME,
@@ -99,7 +102,7 @@ public class MetadataConstraints implements Constraint {
FutureLocationColumnFamily.NAME,
ClonedColumnFamily.NAME,
ExternalCompactionColumnFamily.NAME,
- UpgraderDeprecatedConstants.ChoppedColumnFamily.NAME,
+ CHOPPED,
MergedColumnFamily.NAME
);
// @formatter:on
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java
b/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java
index 5df5668804..fe04b78eb1 100644
---
a/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java
+++
b/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java
@@ -130,12 +130,11 @@ public class ServerContextTest {
});
}
+ // ensure upgrades fail with older, unsupported versions, but pass with
supported versions
@Test
public void testCanRun() {
- // ensure this fails with older versions; the oldest supported version is
hard-coded here
- // to ensure we don't unintentionally break upgrade support; changing this
should be a conscious
- // decision and this check will ensure we don't overlook it
- final int oldestSupported = 10; //
AccumuloDataVersion.ROOT_TABLET_META_CHANGES;
+ final int oldestSupported = AccumuloDataVersion.oldestUpgradeableVersion();
+ assertEquals(10, oldestSupported); // make sure it hasn't changed
accidentally
final int currentVersion = AccumuloDataVersion.get();
IntConsumer shouldPass = ServerContext::ensureDataVersionCompatible;
IntConsumer shouldFail = v -> assertThrows(IllegalStateException.class,
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader11to12.java
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader11to12.java
index 90a6eb4b62..c638d45315 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader11to12.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader11to12.java
@@ -40,10 +40,10 @@ import org.apache.accumulo.core.fate.zookeeper.ZooUtil;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.schema.Ample;
import org.apache.accumulo.core.metadata.schema.MetadataSchema;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ChoppedColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ExternalCompactionColumnFamily;
import org.apache.accumulo.core.metadata.schema.RootTabletMetadata;
-import
org.apache.accumulo.core.metadata.schema.UpgraderDeprecatedConstants.ChoppedColumnFamily;
import org.apache.accumulo.core.security.Authorizations;
import org.apache.accumulo.core.util.TextUtil;
import org.apache.accumulo.server.ServerContext;
@@ -62,9 +62,12 @@ public class Upgrader11to12 implements Upgrader {
private static final Logger log =
LoggerFactory.getLogger(Upgrader11to12.class);
+ @SuppressWarnings("deprecation")
+ private static final Text CHOPPED = ChoppedColumnFamily.NAME;
+
@VisibleForTesting
- static final Set<Text> UPGRADE_FAMILIES = Set.of(DataFileColumnFamily.NAME,
- ChoppedColumnFamily.NAME, ExternalCompactionColumnFamily.NAME);
+ static final Set<Text> UPGRADE_FAMILIES =
+ Set.of(DataFileColumnFamily.NAME, CHOPPED,
ExternalCompactionColumnFamily.NAME);
@Override
public void upgradeZookeeper(@NonNull ServerContext context) {
@@ -165,12 +168,11 @@ public class Upgrader11to12 implements Upgrader {
var family = key.getColumnFamily();
if (family.equals(DataFileColumnFamily.NAME)) {
upgradeDataFileCF(key, value, update);
- } else if (family.equals(ChoppedColumnFamily.NAME)) {
+ } else if (family.equals(CHOPPED)) {
log.warn(
"Deleting chopped reference from:{}. Previous split or delete
may not have completed cleanly. Ref: {}",
tableName, key.getRow());
-
update.at().family(ChoppedColumnFamily.STR_NAME).qualifier(ChoppedColumnFamily.STR_NAME)
- .delete();
+ update.at().family(CHOPPED).qualifier(CHOPPED).delete();
} else if (family.equals(ExternalCompactionColumnFamily.NAME)) {
log.debug(
"Deleting external compaction reference from:{}. Previous
compaction may not have completed. Ref: {}",
diff --git
a/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader11to12Test.java
b/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader11to12Test.java
index 9fedfe5813..7818164e75 100644
---
a/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader11to12Test.java
+++
b/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader11to12Test.java
@@ -51,11 +51,11 @@ import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.Value;
import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter;
import org.apache.accumulo.core.metadata.StoredTabletFile;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ChoppedColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ExternalCompactionColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily;
import org.apache.accumulo.core.metadata.schema.RootTabletMetadata;
-import
org.apache.accumulo.core.metadata.schema.UpgraderDeprecatedConstants.ChoppedColumnFamily;
import org.apache.accumulo.server.ServerContext;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.Text;
@@ -123,8 +123,9 @@ public class Upgrader11to12Test {
Value value2 = new Value("321,654");
scanData.put(key2, value2);
- Key chop1 = Key.builder(false).row(row1).family(ChoppedColumnFamily.NAME)
- .qualifier(ChoppedColumnFamily.NAME).build();
+ @SuppressWarnings("deprecation")
+ var chopped = ChoppedColumnFamily.NAME;
+ Key chop1 =
Key.builder(false).row(row1).family(chopped).qualifier(chopped).build();
scanData.put(chop1, null);
Key extern1 =
Key.builder(false).row(row1).family(ExternalCompactionColumnFamily.NAME)
