This is an automated email from the ASF dual-hosted git repository.
cshannon 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 8fe933a671 Add a constraint check for suspend column (#4546)
8fe933a671 is described below
commit 8fe933a6710066bb88c63401d12e9ff0d88cdf0f
Author: Christopher L. Shannon <cshan...@apache.org>
AuthorDate: Fri May 17 16:40:22 2024 -0400
Add a constraint check for suspend column (#4546)
This adds a new metadata constraint check for the suspend metadata
column to ensure that the suspension time is not negative as we should
never have a negative value.
This is a follow on to #4494
---
.../server/constraints/MetadataConstraints.java | 66 +++++++++++++---------
.../constraints/MetadataConstraintsTest.java | 33 +++++++++++
2 files changed, 72 insertions(+), 27 deletions(-)
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 0f4a361312..1b99a5b307 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
@@ -36,6 +36,7 @@ import org.apache.accumulo.core.fate.zookeeper.ZooUtil;
import org.apache.accumulo.core.lock.ServiceLock;
import org.apache.accumulo.core.metadata.AccumuloTable;
import org.apache.accumulo.core.metadata.StoredTabletFile;
+import org.apache.accumulo.core.metadata.SuspendingTServer;
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;
@@ -306,40 +307,49 @@ public class MetadataConstraints implements Constraint {
} else {
if (!isValidColumn(columnUpdate)) {
violations = addViolation(violations, 2);
- } else if (new
ColumnFQ(columnUpdate).equals(TabletColumnFamily.PREV_ROW_COLUMN)
- && columnUpdate.getValue().length > 0
- && (violations == null || !violations.contains((short) 4))) {
- KeyExtent ke = KeyExtent.fromMetaRow(new Text(mutation.getRow()));
+ } 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()));
+ Text per = TabletColumnFamily.decodePrevEndRow(new
Value(columnUpdate.getValue()));
- boolean prevEndRowLessThanEndRow =
- per == null || ke.endRow() == null || per.compareTo(ke.endRow())
< 0;
+ boolean prevEndRowLessThanEndRow =
+ per == null || ke.endRow() == null ||
per.compareTo(ke.endRow()) < 0;
- if (!prevEndRowLessThanEndRow) {
- violations = addViolation(violations, 3);
- }
- } else if (new
ColumnFQ(columnUpdate).equals(ServerColumnFamily.LOCK_COLUMN)) {
- if (zooCache == null) {
- zooCache = new ZooCache(context.getZooReader(), null);
- CleanerUtil.zooCacheClearer(this, zooCache);
- }
+ 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();
- }
+ if (zooRoot == null) {
+ zooRoot = context.getZooKeeperRoot();
+ }
- boolean lockHeld = false;
- String lockId = new String(columnUpdate.getValue(), UTF_8);
+ 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());
- }
+ 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);
+ 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);
+ }
}
}
}
@@ -379,6 +389,8 @@ public class MetadataConstraints implements Constraint {
return "Bulk load mutation contains either inconsistent files or
multiple fateTX ids";
case 9:
return "Invalid data file metadata format";
+ case 10:
+ return "Suspended timestamp is not valid";
}
return null;
}
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java
b/server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java
index b209a01f76..b7e2bd2a11 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
@@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertNull;
import java.lang.reflect.Method;
import java.util.Base64;
import java.util.List;
+import java.util.concurrent.TimeUnit;
import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.Mutation;
@@ -32,19 +33,25 @@ import org.apache.accumulo.core.data.Range;
import org.apache.accumulo.core.data.Value;
import org.apache.accumulo.core.metadata.AccumuloTable;
import org.apache.accumulo.core.metadata.StoredTabletFile;
+import org.apache.accumulo.core.metadata.SuspendingTServer;
+import org.apache.accumulo.core.metadata.TServerInstance;
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.CurrentLocationColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily;
+import org.apache.accumulo.core.util.time.SteadyTime;
import org.apache.accumulo.server.ServerContext;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.Text;
import org.easymock.EasyMock;
import org.junit.jupiter.api.Test;
+import com.google.common.net.HostAndPort;
+
public class MetadataConstraintsTest {
private SystemEnvironment createEnv() {
@@ -130,6 +137,32 @@ public class MetadataConstraintsTest {
}
+ @Test
+ public void testSuspensionCheck() {
+ Mutation m = new Mutation(new Text("0;foo"));
+ MetadataConstraints mc = new MetadataConstraints();
+ TServerInstance ser1 = new
TServerInstance(HostAndPort.fromParts("server1", 8555), "s001");
+
+ SuspendLocationColumn.SUSPEND_COLUMN.put(m, SuspendingTServer.toValue(ser1,
+ SteadyTime.from(System.currentTimeMillis(), TimeUnit.MILLISECONDS)));
+ List<Short> violations = mc.check(createEnv(), m);
+ assertNull(violations);
+
+ 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);
+
+ 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);
+ assertEquals(1, violations.size());
+ assertEquals(Short.valueOf((short) 10), violations.get(0));
+ }
+
@Test
public void testBulkFileCheck() {
MetadataConstraints mc = new MetadataConstraints();
