This is an automated email from the ASF dual-hosted git repository.
krathbun pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/3.1 by this push:
new de2ae7fd3d Reorganizes MetadataConstraints (#4852)
de2ae7fd3d is described below
commit de2ae7fd3dc1b8b39e012c213b9c9e05dd78509e
Author: Kevin Rathbun <[email protected]>
AuthorDate: Mon Sep 16 10:26:00 2024 -0400
Reorganizes MetadataConstraints (#4852)
- new class `BulkFileColData` to store the data needed to validate a
BulkFileColumn update
Changes to check():
- now uses switch instead of if
- validation logic of each column now done in separate methods
- removed unnecessary checks and simplified logic where applicable (e.g.,
if checks that would never be true, same check done more than once)
- violations are no longer added to and returned by `validate` methods,
only added to. Avoids confusion with how the methods should be used.
- initialized violations at start as a list instead of dealing with
null. Null added extra unnecessary complexity.
---
.../server/constraints/BulkFileColData.java | 77 ++++
.../server/constraints/MetadataConstraints.java | 443 +++++++++++----------
.../server/metadata/RootTabletMutatorImpl.java | 2 +-
.../constraints/MetadataConstraintsTest.java | 41 +-
.../apache/accumulo/test/functional/BulkNewIT.java | 3 +-
5 files changed, 342 insertions(+), 224 deletions(-)
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/constraints/BulkFileColData.java
b/server/base/src/main/java/org/apache/accumulo/server/constraints/BulkFileColData.java
new file mode 100644
index 0000000000..6533f8a19a
--- /dev/null
+++
b/server/base/src/main/java/org/apache/accumulo/server/constraints/BulkFileColData.java
@@ -0,0 +1,77 @@
+/*
+ * 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.constraints;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.accumulo.core.metadata.StoredTabletFile;
+
+/**
+ * Data needed to validate a BulkFileColumn update
+ */
+class BulkFileColData {
+ private boolean isSplitMutation, isLocationMutation;
+ private final Set<StoredTabletFile> dataFiles, loadedFiles;
+ private final Set<String> tidsSeen;
+
+ BulkFileColData() {
+ isSplitMutation = false;
+ isLocationMutation = false;
+ dataFiles = new HashSet<>();
+ loadedFiles = new HashSet<>();
+ tidsSeen = new HashSet<>();
+ }
+
+ void setIsSplitMutation(boolean b) {
+ isSplitMutation = b;
+ }
+
+ boolean getIsSplitMutation() {
+ return isSplitMutation;
+ }
+
+ void setIsLocationMutation(boolean b) {
+ isLocationMutation = b;
+ }
+
+ boolean getIsLocationMutation() {
+ return isLocationMutation;
+ }
+
+ void addDataFile(StoredTabletFile stf) {
+ dataFiles.add(stf);
+ }
+
+ void addLoadedFile(StoredTabletFile stf) {
+ loadedFiles.add(stf);
+ }
+
+ boolean dataFilesEqualsLoadedFiles() {
+ return dataFiles.equals(loadedFiles);
+ }
+
+ void addTidSeen(String tid) {
+ tidsSeen.add(tid);
+ }
+
+ Set<String> getTidsSeen() {
+ return tidsSeen;
+ }
+}
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 da7acdda64..c3ca7bdd4a 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
@@ -21,10 +21,11 @@ package org.apache.accumulo.server.constraints;
import static java.nio.charset.StandardCharsets.UTF_8;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
-import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import java.util.function.Consumer;
import org.apache.accumulo.core.data.ColumnUpdate;
import org.apache.accumulo.core.data.Mutation;
@@ -62,6 +63,7 @@ import org.slf4j.LoggerFactory;
public class MetadataConstraints implements Constraint {
private static final Logger log =
LoggerFactory.getLogger(MetadataConstraints.class);
+ private static final byte[] BULK_COL_BYTES =
BulkFileColumnFamily.STR_NAME.getBytes(UTF_8);
private ZooCache zooCache = null;
private String zooRoot = null;
@@ -113,246 +115,115 @@ public class MetadataConstraints implements Constraint {
return validColumnQuals.contains(new ColumnFQ(cu));
}
- private static ArrayList<Short> addViolation(ArrayList<Short> lst, int
violation) {
- if (lst == null) {
- lst = new ArrayList<>();
- }
+ private static void addViolation(ArrayList<Short> lst, int violation) {
lst.add((short) violation);
- return lst;
}
- private static ArrayList<Short> addIfNotPresent(ArrayList<Short> lst, int
intViolation) {
- if (lst == null) {
- return addViolation(null, intViolation);
- }
- short violation = (short) intViolation;
- if (!lst.contains(violation)) {
- return addViolation(lst, intViolation);
+ private static void addIfNotPresent(ArrayList<Short> lst, int violation) {
+ if (!lst.contains((short) violation)) {
+ addViolation(lst, violation);
}
- return lst;
}
/*
* Validates the data file metadata is valid for a StoredTabletFile.
*/
- private static ArrayList<Short> validateDataFileMetadata(ArrayList<Short>
violations,
- String metadata) {
+ private static void validateDataFileMetadata(ArrayList<Short> violations,
String metadata,
+ Consumer<StoredTabletFile> stfConsumer) {
try {
- StoredTabletFile.validate(metadata);
+ stfConsumer.accept(StoredTabletFile.of(metadata));
} catch (RuntimeException e) {
- violations = addViolation(violations, 9);
+ addViolation(violations, 9);
}
- return violations;
}
+ /**
+ * Same as defined in {@link Constraint#check(Environment, Mutation)}, but
returns an empty list
+ * instead of null if there are no violations
+ *
+ * @param env constraint environment
+ * @param mutation mutation to check
+ * @return list of violations, or empty list if no violations
+ */
@Override
public List<Short> check(Environment env, Mutation mutation) {
final ServerContext context = ((SystemEnvironment) env).getServerContext();
-
- ArrayList<Short> violations = null;
-
- Collection<ColumnUpdate> colUpdates = mutation.getUpdates();
-
- // check the row, it should contains at least one ; or end with <
- boolean containsSemiC = false;
-
- byte[] row = mutation.getRow();
-
- // always allow rows that fall within reserved areas
- if (row.length > 0 && row[0] == '~') {
- return null;
- }
-
- for (byte b : row) {
- if (b == ';') {
- containsSemiC = true;
- }
-
- if (b == ';' || b == '<') {
- break;
- }
-
- if (!validTableNameChars[0xff & b]) {
- violations = addIfNotPresent(violations, 4);
- }
- }
-
- if (containsSemiC) {
- if (row.length == 0) {
- violations = addIfNotPresent(violations, 4);
- }
+ final ArrayList<Short> violations = new ArrayList<>();
+ final Collection<ColumnUpdate> colUpdates = mutation.getUpdates();
+ final byte[] row = mutation.getRow();
+ final List<ColumnUpdate> bulkFileColUpdates;
+ final BulkFileColData bfcValidationData;
+
+ // avoids creating unnecessary objects
+ if (hasBulkCol(colUpdates)) {
+ bulkFileColUpdates = new ArrayList<>();
+ bfcValidationData = new BulkFileColData();
} else {
- // see if last row char is <
- if (row.length == 0 || row[row.length - 1] != '<') {
- violations = addIfNotPresent(violations, 4);
- }
+ bulkFileColUpdates = null;
+ bfcValidationData = null;
}
- if (row.length > 0 && row[0] == '!') {
- if (row.length < 3 || row[1] != '0' || (row[2] != '<' && row[2] != ';'))
{
- violations = addIfNotPresent(violations, 4);
- }
- }
-
- // ensure row is not less than Constants.METADATA_TABLE_ID
- if (new Text(row).compareTo(new
Text(AccumuloTable.METADATA.tableId().canonical())) < 0) {
- violations = addViolation(violations, 5);
+ // always allow rows that fall within reserved areas
+ if (row.length > 0 && row[0] == '~') {
+ return violations;
}
- boolean checkedBulk = false;
+ validateTabletRow(violations, row);
for (ColumnUpdate columnUpdate : colUpdates) {
- Text columnFamily = new Text(columnUpdate.getColumnFamily());
+ String colFamStr = new String(columnUpdate.getColumnFamily(), UTF_8);
if (columnUpdate.isDeleted()) {
if (!isValidColumn(columnUpdate)) {
- violations = addViolation(violations, 2);
+ addViolation(violations, 2);
}
continue;
}
- if (columnUpdate.getValue().length == 0 &&
!(columnFamily.equals(ScanFileColumnFamily.NAME)
- || columnFamily.equals(LogColumnFamily.NAME))) {
- violations = addViolation(violations, 6);
- }
-
- if (columnFamily.equals(DataFileColumnFamily.NAME)) {
- violations = validateDataFileMetadata(violations,
- new String(columnUpdate.getColumnQualifier(), UTF_8));
-
- try {
- DataFileValue dfv = new DataFileValue(columnUpdate.getValue());
+ validateColValLen(violations, columnUpdate);
- if (dfv.getSize() < 0 || dfv.getNumEntries() < 0) {
- violations = addViolation(violations, 1);
- }
- } catch (NumberFormatException | ArrayIndexOutOfBoundsException nfe) {
- violations = addViolation(violations, 1);
- }
- } else if (columnFamily.equals(ScanFileColumnFamily.NAME)) {
- violations = validateDataFileMetadata(violations,
- new String(columnUpdate.getColumnQualifier(), UTF_8));
- } else if (columnFamily.equals(BulkFileColumnFamily.NAME)) {
- if (!columnUpdate.isDeleted() && !checkedBulk) {
- /*
- * This needs to be re-worked after Issue
https://github.com/apache/accumulo/issues/3505
- * is done.
- *
- * That issue will reorganizes this class and make things more
efficient so we are not
- * looping over the same mutation more than once like in this case.
The below check is
- * commented out for now because the violation check is already done
when creating
- * StoredTabletFiles so it isn't needed here anymore violations =
- * validateDataFileMetadata(violations, new
String(columnUpdate.getColumnQualifier(),
- * UTF_8));
- */
-
- // splits, which also write the time reference, are allowed to write
this reference even
- // when
- // the transaction is not running because the other half of the
tablet is holding a
- // reference
- // to the file.
- boolean isSplitMutation = false;
+ switch (colFamStr) {
+ case TabletColumnFamily.STR_NAME:
+ validateTabletFamily(violations, columnUpdate, mutation);
+ break;
+ case ServerColumnFamily.STR_NAME:
+ validateServerFamily(violations, columnUpdate, context,
bfcValidationData);
+ break;
+ case CurrentLocationColumnFamily.STR_NAME:
// When a tablet is assigned, it re-writes the metadata. It should
probably only update
- // the location information,
- // but it writes everything. We allow it to re-write the bulk
information if it is setting
- // the location.
+ // the location information, but it writes everything. We allow it
to re-write the bulk
+ // information if it is setting the location.
// See ACCUMULO-1230.
- boolean isLocationMutation = false;
-
- HashSet<StoredTabletFile> dataFiles = new HashSet<>();
- HashSet<StoredTabletFile> loadedFiles = new HashSet<>();
-
- String tidString = new String(columnUpdate.getValue(), UTF_8);
- int otherTidCount = 0;
-
- for (ColumnUpdate update : mutation.getUpdates()) {
- if (new
ColumnFQ(update).equals(ServerColumnFamily.DIRECTORY_COLUMN)) {
- isSplitMutation = true;
- } else if (new Text(update.getColumnFamily())
- .equals(CurrentLocationColumnFamily.NAME)) {
- isLocationMutation = true;
- } else if (new
Text(update.getColumnFamily()).equals(DataFileColumnFamily.NAME)) {
- try {
- // This actually validates for a second time as the loop
already validates
- // if a DataFileColumnFamily, this will likely be fixed as
part of
- // https://github.com/apache/accumulo/issues/3505
- dataFiles.add(StoredTabletFile.of(new
Text(update.getColumnQualifier())));
- } catch (RuntimeException e) {
- violations = addViolation(violations, 9);
- }
- } else if (new
Text(update.getColumnFamily()).equals(BulkFileColumnFamily.NAME)) {
- try {
- loadedFiles.add(StoredTabletFile.of(new
Text(update.getColumnQualifier())));
- } catch (RuntimeException e) {
- violations = addViolation(violations, 9);
- }
-
- if (!new String(update.getValue(), UTF_8).equals(tidString)) {
- otherTidCount++;
- }
- }
+ if (bfcValidationData != null) {
+ bfcValidationData.setIsLocationMutation(true);
}
-
- if (!isSplitMutation && !isLocationMutation) {
- if (otherTidCount > 0 || !dataFiles.equals(loadedFiles)) {
- violations = addViolation(violations, 8);
- }
+ break;
+ case SuspendLocationColumn.STR_NAME:
+ validateSuspendLocationFamily(violations, columnUpdate);
+ break;
+ case BulkFileColumnFamily.STR_NAME:
+ // defer validating the bulk file column updates until the end
(relies on checks done
+ // on other column updates)
+ if (bulkFileColUpdates != null) {
+ bulkFileColUpdates.add(columnUpdate);
}
-
- checkedBulk = true;
- }
- } else {
- if (!isValidColumn(columnUpdate)) {
- violations = addViolation(violations, 2);
- } else {
- final var column = new ColumnFQ(columnUpdate);
- if (column.equals(TabletColumnFamily.PREV_ROW_COLUMN)
- && columnUpdate.getValue().length > 0
- && (violations == null || !violations.contains((short) 4))) {
- KeyExtent ke = KeyExtent.fromMetaRow(new Text(mutation.getRow()));
-
- Text per = TabletColumnFamily.decodePrevEndRow(new
Value(columnUpdate.getValue()));
-
- boolean prevEndRowLessThanEndRow =
- per == null || ke.endRow() == null ||
per.compareTo(ke.endRow()) < 0;
-
- if (!prevEndRowLessThanEndRow) {
- violations = addViolation(violations, 3);
- }
- } else if (column.equals(ServerColumnFamily.LOCK_COLUMN)) {
- if (zooCache == null) {
- zooCache = new ZooCache(context.getZooReader(), null);
- CleanerUtil.zooCacheClearer(this, zooCache);
- }
-
- if (zooRoot == null) {
- zooRoot = context.getZooKeeperRoot();
- }
-
- boolean lockHeld = false;
- String lockId = new String(columnUpdate.getValue(), UTF_8);
-
- try {
- lockHeld = ServiceLock.isLockHeld(zooCache, new
ZooUtil.LockID(zooRoot, lockId));
- } catch (Exception e) {
- log.debug("Failed to verify lock was held {} {}", lockId,
e.getMessage());
- }
-
- if (!lockHeld) {
- violations = addViolation(violations, 7);
- }
- } else if (column.equals(SuspendLocationColumn.SUSPEND_COLUMN)) {
- try {
- SuspendingTServer.fromValue(new Value(columnUpdate.getValue()));
- } catch (IllegalArgumentException e) {
- violations = addViolation(violations, 10);
- }
+ break;
+ case DataFileColumnFamily.STR_NAME:
+ validateDataFileFamily(violations, columnUpdate, bfcValidationData);
+ break;
+ case ScanFileColumnFamily.STR_NAME:
+ validateScanFileFamily(violations, columnUpdate);
+ break;
+ default:
+ if (!isValidColumn(columnUpdate)) {
+ addViolation(violations, 2);
}
- }
}
}
- if (violations != null) {
+ validateBulkFileFamily(violations, bulkFileColUpdates, bfcValidationData);
+
+ if (!violations.isEmpty()) {
log.debug("violating metadata mutation : {}", new
String(mutation.getRow(), UTF_8));
for (ColumnUpdate update : mutation.getUpdates()) {
log.debug(" update: {}:{} value {}", new
String(update.getColumnFamily(), UTF_8),
@@ -392,4 +263,174 @@ public class MetadataConstraints implements Constraint {
return null;
}
+ private void validateColValLen(ArrayList<Short> violations, ColumnUpdate
columnUpdate) {
+ Text columnFamily = new Text(columnUpdate.getColumnFamily());
+ if (columnUpdate.getValue().length == 0 &&
!(columnFamily.equals(ScanFileColumnFamily.NAME)
+ || columnFamily.equals(LogColumnFamily.NAME))) {
+ addViolation(violations, 6);
+ }
+ }
+
+ private void validateTabletRow(ArrayList<Short> violations, byte[] row) {
+ // check the row, it should contain at least one ";" or end with "<". Row
should also
+ // not be less than AccumuloTable.METADATA.tableId().
+ boolean containsSemiC = false;
+
+ for (byte b : row) {
+ if (b == ';') {
+ containsSemiC = true;
+ }
+
+ if (b == ';' || b == '<') {
+ break;
+ }
+
+ if (!validTableNameChars[0xff & b]) {
+ addIfNotPresent(violations, 4);
+ }
+ }
+
+ if (!containsSemiC) {
+ // see if last row char is <
+ if (row.length == 0 || row[row.length - 1] != '<') {
+ addIfNotPresent(violations, 4);
+ }
+ }
+
+ if (row.length > 0 && row[0] == '!') {
+ if (row.length < 3 || row[1] != '0' || (row[2] != '<' && row[2] != ';'))
{
+ addIfNotPresent(violations, 4);
+ }
+ }
+
+ // ensure row is not less than AccumuloTable.METADATA.tableId()
+ if (Arrays.compare(row,
AccumuloTable.METADATA.tableId().canonical().getBytes(UTF_8)) < 0) {
+ addViolation(violations, 5);
+ }
+ }
+
+ private void validateTabletFamily(ArrayList<Short> violations, ColumnUpdate
columnUpdate,
+ Mutation mutation) {
+ String qualStr = new String(columnUpdate.getColumnQualifier(), UTF_8);
+
+ if (qualStr.equals(TabletColumnFamily.PREV_ROW_QUAL)) {
+ if (columnUpdate.getValue().length > 0 && !violations.contains((short)
4)) {
+ KeyExtent ke = KeyExtent.fromMetaRow(new Text(mutation.getRow()));
+ Text per = TabletColumnFamily.decodePrevEndRow(new
Value(columnUpdate.getValue()));
+ boolean prevEndRowLessThanEndRow =
+ per == null || ke.endRow() == null || per.compareTo(ke.endRow()) <
0;
+
+ if (!prevEndRowLessThanEndRow) {
+ addViolation(violations, 3);
+ }
+ }
+ }
+ }
+
+ private void validateServerFamily(ArrayList<Short> violations, ColumnUpdate
columnUpdate,
+ ServerContext context, BulkFileColData bfcValidationData) {
+ String qualStr = new String(columnUpdate.getColumnQualifier(), UTF_8);
+
+ switch (qualStr) {
+ case ServerColumnFamily.LOCK_QUAL:
+ if (zooCache == null) {
+ zooCache = new ZooCache(context.getZooReader(), null);
+ CleanerUtil.zooCacheClearer(this, zooCache);
+ }
+
+ if (zooRoot == null) {
+ zooRoot = context.getZooKeeperRoot();
+ }
+
+ boolean lockHeld = false;
+ String lockId = new String(columnUpdate.getValue(), UTF_8);
+
+ try {
+ lockHeld = ServiceLock.isLockHeld(zooCache, new
ZooUtil.LockID(zooRoot, lockId));
+ } catch (Exception e) {
+ log.debug("Failed to verify lock was held {} {}", lockId,
e.getMessage());
+ }
+
+ if (!lockHeld) {
+ addViolation(violations, 7);
+ }
+ break;
+ case ServerColumnFamily.DIRECTORY_QUAL:
+ // splits, which also write the time reference, are allowed to write
this reference
+ // even when the transaction is not running because the other half of
the tablet is
+ // holding a reference to the file.
+ if (bfcValidationData != null) {
+ bfcValidationData.setIsSplitMutation(true);
+ }
+ break;
+ }
+ }
+
+ private void validateSuspendLocationFamily(ArrayList<Short> violations,
+ ColumnUpdate columnUpdate) {
+ String qualStr = new String(columnUpdate.getColumnQualifier(), UTF_8);
+ String suspendColQualStr =
+ new
String(SuspendLocationColumn.SUSPEND_COLUMN.getColumnQualifier().getBytes(),
UTF_8);
+
+ if (qualStr.equals(suspendColQualStr)) {
+ try {
+ SuspendingTServer.fromValue(new Value(columnUpdate.getValue()));
+ } catch (IllegalArgumentException e) {
+ addViolation(violations, 10);
+ }
+ }
+ }
+
+ private void validateDataFileFamily(ArrayList<Short> violations,
ColumnUpdate columnUpdate,
+ BulkFileColData bfcValidationData) {
+ Consumer<StoredTabletFile> stfConsumer =
+ bfcValidationData == null ? stf -> {} : bfcValidationData::addDataFile;
+ validateDataFileMetadata(violations, new
String(columnUpdate.getColumnQualifier(), UTF_8),
+ stfConsumer);
+
+ try {
+ DataFileValue dfv = new DataFileValue(columnUpdate.getValue());
+
+ if (dfv.getSize() < 0 || dfv.getNumEntries() < 0) {
+ addViolation(violations, 1);
+ }
+ } catch (NumberFormatException | ArrayIndexOutOfBoundsException nfe) {
+ addViolation(violations, 1);
+ }
+ }
+
+ private void validateScanFileFamily(ArrayList<Short> violations,
ColumnUpdate columnUpdate) {
+ validateDataFileMetadata(violations, new
String(columnUpdate.getColumnQualifier(), UTF_8),
+ stf -> {});
+ }
+
+ private void validateBulkFileFamily(ArrayList<Short> violations,
+ Collection<ColumnUpdate> bulkFileColUpdates, BulkFileColData
bfcValidationData) {
+ if (bulkFileColUpdates != null && !bulkFileColUpdates.isEmpty()) {
+ for (ColumnUpdate bulkFileColUpdate : bulkFileColUpdates) {
+ validateDataFileMetadata(violations,
+ new String(bulkFileColUpdate.getColumnQualifier(), UTF_8),
+ bfcValidationData::addLoadedFile);
+
+ bfcValidationData.addTidSeen(new String(bulkFileColUpdate.getValue(),
UTF_8));
+ }
+
+ if (!bfcValidationData.getIsSplitMutation() &&
!bfcValidationData.getIsLocationMutation()) {
+ if (bfcValidationData.getTidsSeen().size() > 1
+ || !bfcValidationData.dataFilesEqualsLoadedFiles()) {
+ addViolation(violations, 8);
+ }
+ }
+ }
+ }
+
+ private boolean hasBulkCol(Collection<ColumnUpdate> colUpdates) {
+ for (ColumnUpdate colUpdate : colUpdates) {
+ if (Arrays.equals(colUpdate.getColumnFamily(), BULK_COL_BYTES)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/metadata/RootTabletMutatorImpl.java
b/server/base/src/main/java/org/apache/accumulo/server/metadata/RootTabletMutatorImpl.java
index 7ebdd7db92..85c5992b4f 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/metadata/RootTabletMutatorImpl.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/metadata/RootTabletMutatorImpl.java
@@ -83,7 +83,7 @@ public class RootTabletMutatorImpl extends TabletMutatorBase
implements Ample.Ta
MetadataConstraints metaConstraint = new MetadataConstraints();
List<Short> violations = metaConstraint.check(new RootEnv(context),
mutation);
- if (violations != null && !violations.isEmpty()) {
+ if (!violations.isEmpty()) {
throw new IllegalStateException(
"Mutation for root tablet metadata violated constraints : " +
violations);
}
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 b8e7cf38a1..4d69ebd8a6 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,9 @@
package org.apache.accumulo.server.constraints;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.lang.reflect.Method;
import java.util.Base64;
@@ -71,7 +72,7 @@ public class MetadataConstraintsTest {
List<Short> violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 3), violations.get(0));
@@ -80,7 +81,7 @@ public class MetadataConstraintsTest {
violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 4), violations.get(0));
@@ -89,7 +90,7 @@ public class MetadataConstraintsTest {
violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 2), violations.get(0));
@@ -98,7 +99,7 @@ public class MetadataConstraintsTest {
violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(2, violations.size());
assertEquals(Short.valueOf((short) 4), violations.get(0));
assertEquals(Short.valueOf((short) 5), violations.get(1));
@@ -108,7 +109,7 @@ public class MetadataConstraintsTest {
violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 6), violations.get(0));
@@ -117,21 +118,21 @@ public class MetadataConstraintsTest {
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
m = new Mutation(new Text(AccumuloTable.METADATA.tableId().canonical() +
"<"));
TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("bar"));
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
m = new Mutation(new Text("!1<"));
TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("bar"));
violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 4), violations.get(0));
@@ -146,19 +147,19 @@ public class MetadataConstraintsTest {
SuspendLocationColumn.SUSPEND_COLUMN.put(m, SuspendingTServer.toValue(ser1,
SteadyTime.from(System.currentTimeMillis(), TimeUnit.MILLISECONDS)));
List<Short> violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
m = new Mutation(new Text("0;foo"));
SuspendLocationColumn.SUSPEND_COLUMN.put(m,
SuspendingTServer.toValue(ser1, SteadyTime.from(0,
TimeUnit.MILLISECONDS)));
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
m = new Mutation(new Text("0;foo"));
// We must encode manually since SteadyTime won't allow a negative
SuspendLocationColumn.SUSPEND_COLUMN.put(m, new Value(ser1.getHostPort() +
"|" + -1L));
violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 10), violations.get(0));
}
@@ -180,7 +181,7 @@ public class MetadataConstraintsTest {
.of(new
Path("hdfs://1.2.3.4/accumulo/tables/2a/t-0003/someFile")).getMetadataText(),
new DataFileValue(1, 1).encodeAsValue());
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
// loaded marker w/o file
m = new Mutation(new Text("0;foo"));
@@ -209,7 +210,7 @@ public class MetadataConstraintsTest {
.of(new
Path("hdfs://1.2.3.4/accumulo/tables/2a/t-0003/someFile2")).getMetadataText(),
new DataFileValue(1, 1).encodeAsValue());
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
// two files w/ different txid
m = new Mutation(new Text("0;foo"));
@@ -255,7 +256,7 @@ public class MetadataConstraintsTest {
new Value("5"));
ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value("/t1"));
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
// mutation that looks like a load
m = new Mutation(new Text("0;foo"));
@@ -265,14 +266,14 @@ public class MetadataConstraintsTest {
new Value("5"));
m.put(CurrentLocationColumnFamily.NAME, new Text("789"), new
Value("127.0.0.1:9997"));
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
// deleting a load flag
m = new Mutation(new Text("0;foo"));
m.putDelete(BulkFileColumnFamily.NAME, StoredTabletFile
.of(new
Path("hdfs://1.2.3.4/accumulo/tables/2a/t-0003/someFile")).getMetadataText());
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
// Missing beginning of path
m = new Mutation(new Text("0;foo"));
@@ -460,7 +461,7 @@ public class MetadataConstraintsTest {
.of(new
Path("hdfs://1.2.3.4/accumulo/tables/2a/t-0003/someFile")).getMetadataText(),
new DataFileValue(1, 1).encodeAsValue());
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
// Should pass validation with range set
m = new Mutation(new Text("0;foo"));
@@ -469,7 +470,7 @@ public class MetadataConstraintsTest {
new Range("a", false, "b", true)).getMetadataText(),
new DataFileValue(1, 1).encodeAsValue());
violations = mc.check(createEnv(), m);
- assertNull(violations);
+ assertTrue(violations.isEmpty());
assertNotNull(mc.getViolationDescription((short) 9));
}
@@ -488,7 +489,7 @@ public class MetadataConstraintsTest {
private void assertViolation(MetadataConstraints mc, Mutation m, Short
violation) {
List<Short> violations = mc.check(createEnv(), m);
- assertNotNull(violations);
+ assertFalse(violations.isEmpty());
assertEquals(1, violations.size());
assertEquals(violation, violations.get(0));
assertNotNull(mc.getViolationDescription(violations.get(0)));
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java
index 06fdb6945f..ee9d418b68 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java
@@ -24,7 +24,6 @@ import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType
import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.PREV_ROW;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -723,7 +722,7 @@ public class BulkNewIT extends SharedMiniClusterBase {
m.put("", "", NoBulkConstratint.CANARY_VALUE);
// This test assume the metadata constraint check will not flag this
mutation, the following
// validates this assumption.
- assertNull(metaConstraints.check(env, m));
+ assertTrue(metaConstraints.check(env, m).isEmpty());
bw.addMutation(m);
return false;
} catch (MutationsRejectedException e) {
