You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by kt...@apache.org on 2023/05/01 14:54:38 UTC
[accumulo] branch elasticity updated: improve code and docs related to operation id column (#3364)
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 3e2ed1be89 improve code and docs related to operation id column (#3364)
3e2ed1be89 is described below
commit 3e2ed1be897936c192214c2f08f7b2cfc6c1eeea
Author: Keith Turner <kt...@apache.org>
AuthorDate: Mon May 1 10:54:32 2023 -0400
improve code and docs related to operation id column (#3364)
---
.../accumulo/core/metadata/schema/Ample.java | 10 ++---
.../core/metadata/schema/MetadataSchema.java | 9 +++++
.../core/metadata/schema/TabletMetadata.java | 15 +++-----
.../metadata/{ => schema}/TabletOperationId.java | 37 +++++++++++++++++-
...bletOperation.java => TabletOperationType.java} | 2 +-
.../server/constraints/MetadataConstraints.java | 14 ++++++-
.../metadata/ConditionalTabletMutatorImpl.java | 8 ++--
.../server/metadata/TabletMutatorBase.java | 7 ++--
.../constraints/MetadataConstraintsTest.java | 19 ++++++++++
.../test/functional/AmpleConditionalWriterIT.java | 44 ++++++++++------------
10 files changed, 112 insertions(+), 53 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java
index 438e78b9e1..d7a7177da2 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java
@@ -37,7 +37,6 @@ import org.apache.accumulo.core.metadata.ScanServerRefTabletFile;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.TServerInstance;
import org.apache.accumulo.core.metadata.TabletFile;
-import org.apache.accumulo.core.metadata.TabletOperationId;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location;
import org.apache.accumulo.core.tabletserver.log.LogEntry;
@@ -321,7 +320,7 @@ public interface Ample {
T deleteHostingRequested();
- T putOperation(TabletOperation operation, TabletOperationId opId);
+ T putOperation(TabletOperationId opId);
T deleteOperation();
@@ -347,7 +346,9 @@ public interface Ample {
* A tablet operation is a mutually exclusive action that is running against a tablet. Its very
* important that every conditional mutation specifies requirements about operations in order to
* satisfy the mutual exclusion goal. This interface forces those requirements to specified by
- * making it the only choice avialable before specifying other tablet requirements or mutations.
+ * making it the only choice available before specifying other tablet requirements or mutations.
+ *
+ * @see MetadataSchema.TabletsSection.ServerColumnFamily#OPID_COLUMN
*/
interface OperationRequirements {
@@ -355,8 +356,7 @@ public interface Ample {
* Require a specific operation with a unique id is present. This would be normally be called by
* the code executing that operation.
*/
- ConditionalTabletMutator requireOperation(TabletOperation operation,
- TabletOperationId operationId);
+ ConditionalTabletMutator requireOperation(TabletOperationId operationId);
/**
* Require that no mutually exclusive operations are runnnig against this tablet.
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 f387c6b20c..793d36d41d 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
@@ -249,6 +249,15 @@ public class MetadataSchema {
public static final String LOCK_QUAL = "lock";
public static final ColumnFQ LOCK_COLUMN = new ColumnFQ(NAME, new Text(LOCK_QUAL));
+ /**
+ * This column is used to indicate an operation is running that needs exclusive access to read
+ * and write to a tablet. The value uniquely identifies a FATE operation that is running and
+ * needs the exclusive access. All tablet updates must either ensure this column is absent or
+ * in the case of a FATE operation that set it ensure the value contains their FATE
+ * transaction id. When a FATE operation wants to set this column it must ensure its absent
+ * before setting it. Once a FATE operation has successfully set the column then no other
+ * tablet update should succeed.
+ */
public static final String OPID_QUAL = "opid";
public static final ColumnFQ OPID_COLUMN = new ColumnFQ(NAME, new Text(OPID_QUAL));
}
diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
index 69e85a1139..6a158cd612 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
@@ -61,7 +61,6 @@ import org.apache.accumulo.core.metadata.SuspendingTServer;
import org.apache.accumulo.core.metadata.TServerInstance;
import org.apache.accumulo.core.metadata.TabletFile;
import org.apache.accumulo.core.metadata.TabletLocationState;
-import org.apache.accumulo.core.metadata.TabletOperationId;
import org.apache.accumulo.core.metadata.TabletState;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.BulkFileColumnFamily;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ChoppedColumnFamily;
@@ -119,7 +118,6 @@ public class TabletMetadata {
private boolean chopped = false;
private TabletHostingGoal goal = TabletHostingGoal.ONDEMAND;
private boolean onDemandHostingRequested = false;
- private TabletOperation operation;
private TabletOperationId operationId;
public enum LocationType {
@@ -404,11 +402,10 @@ public class TabletMetadata {
return extCompactions;
}
- public TabletOperation getOperation() {
- ensureFetched(ColumnType.OPID);
- return operation;
- }
-
+ /**
+ * @return the operation id if it exist, null otherwise
+ * @see MetadataSchema.TabletsSection.ServerColumnFamily#OPID_COLUMN
+ */
public TabletOperationId getOperationId() {
ensureFetched(ColumnType.OPID);
return operationId;
@@ -485,9 +482,7 @@ public class TabletMetadata {
te.compact = OptionalLong.of(Long.parseLong(val));
break;
case OPID_QUAL:
- String[] tokens = val.split(":", 2);
- te.operation = TabletOperation.valueOf(tokens[0]);
- te.operationId = new TabletOperationId(tokens[1]);
+ te.operationId = TabletOperationId.from(val);
break;
}
break;
diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/TabletOperationId.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationId.java
similarity index 50%
rename from core/src/main/java/org/apache/accumulo/core/metadata/TabletOperationId.java
rename to core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationId.java
index 322c7e0f1f..49b7cf169a 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/TabletOperationId.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationId.java
@@ -16,9 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.core.metadata;
+package org.apache.accumulo.core.metadata.schema;
import org.apache.accumulo.core.data.AbstractId;
+import org.apache.accumulo.core.fate.FateTxId;
+
+import com.google.common.base.Preconditions;
/**
* Intended to contain a globally unique id that identifies an operation running against a tablet.
@@ -28,7 +31,37 @@ public class TabletOperationId extends AbstractId<TabletOperationId> {
private static final long serialVersionUID = 1L;
- public TabletOperationId(String canonical) {
+ public static String validate(String opid) {
+ var fields = opid.split(":");
+ Preconditions.checkArgument(fields.length == 2, "Malformed operation id %s", opid);
+ try {
+ TabletOperationType.valueOf(fields[0]);
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException("Malformed operation id " + opid, e);
+ }
+
+ if (!FateTxId.isFormatedTid(fields[1])) {
+ throw new IllegalArgumentException("Malformed operation id " + opid);
+ }
+
+ return opid;
+ }
+
+ private TabletOperationId(String canonical) {
super(canonical);
}
+
+ public TabletOperationType getType() {
+ var fields = canonical().split(":");
+ Preconditions.checkState(fields.length == 2);
+ return TabletOperationType.valueOf(fields[0]);
+ }
+
+ public static TabletOperationId from(String opid) {
+ return new TabletOperationId(validate(opid));
+ }
+
+ public static TabletOperationId from(TabletOperationType type, long txid) {
+ return new TabletOperationId(type + ":" + FateTxId.formatTid(txid));
+ }
}
diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperation.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationType.java
similarity index 96%
rename from core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperation.java
rename to core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationType.java
index fd58ddb61b..4565f126b5 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperation.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationType.java
@@ -21,6 +21,6 @@ package org.apache.accumulo.core.metadata.schema;
/**
* Used to specify what kind of mutually exclusive operation is currently running against a tablet.
*/
-public enum TabletOperation {
+public enum TabletOperationType {
SPLITTING, MERGING, DELETING
}
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 26f55b3119..084b1fd932 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
@@ -53,6 +53,7 @@ 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.TabletOperationId;
import org.apache.accumulo.core.util.ColumnFQ;
import org.apache.accumulo.core.util.cleaner.CleanerUtil;
import org.apache.accumulo.server.ServerContext;
@@ -224,8 +225,15 @@ public class MetadataConstraints implements Constraint {
try {
TabletHostingGoalUtil.fromValue(new Value(columnUpdate.getValue()));
} catch (IllegalArgumentException e) {
- violations = addViolation(violations, 4);
+ violations = addViolation(violations, 10);
}
+ } else if (ServerColumnFamily.OPID_COLUMN.equals(columnFamily, columnQualifier)) {
+ try {
+ TabletOperationId.validate(new String(columnUpdate.getValue(), UTF_8));
+ } catch (IllegalArgumentException e) {
+ violations = addViolation(violations, 9);
+ }
+
} else if (columnFamily.equals(BulkFileColumnFamily.NAME)) {
if (!columnUpdate.isDeleted() && !checkedBulk) {
// splits, which also write the time reference, are allowed to write this reference even
@@ -357,6 +365,10 @@ public class MetadataConstraints implements Constraint {
return "Lock not held in zookeeper by writer";
case 8:
return "Bulk load transaction no longer running";
+ case 9:
+ return "Malformed operation id";
+ case 10:
+ return "Malformed hosting goal";
}
return null;
}
diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java
index 517e0b6a36..bd9effe468 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java
@@ -31,13 +31,12 @@ import org.apache.accumulo.core.data.ConditionalMutation;
import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.TabletFile;
-import org.apache.accumulo.core.metadata.TabletOperationId;
import org.apache.accumulo.core.metadata.schema.Ample;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.BulkFileColumnFamily;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
import org.apache.accumulo.core.metadata.schema.TabletMetadata;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location;
-import org.apache.accumulo.core.metadata.schema.TabletOperation;
+import org.apache.accumulo.core.metadata.schema.TabletOperationId;
import org.apache.accumulo.server.ServerContext;
import org.apache.accumulo.server.metadata.iterators.LocationExistsIterator;
import org.apache.accumulo.server.metadata.iterators.PresentIterator;
@@ -133,11 +132,10 @@ public class ConditionalTabletMutatorImpl extends TabletMutatorBase<Ample.Condit
}
@Override
- public Ample.ConditionalTabletMutator requireOperation(TabletOperation operation,
- TabletOperationId opid) {
+ public Ample.ConditionalTabletMutator requireOperation(TabletOperationId opid) {
Preconditions.checkState(updatesEnabled, "Cannot make updates after calling mutate.");
Condition c = new Condition(OPID_COLUMN.getColumnFamily(), OPID_COLUMN.getColumnQualifier())
- .setValue(operation.name() + ":" + opid.canonical());
+ .setValue(opid.canonical());
mutation.addCondition(c);
sawOperationRequirement = true;
return this;
diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
index c56923292b..02bb9b8f62 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
@@ -29,7 +29,6 @@ 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.TabletFile;
-import org.apache.accumulo.core.metadata.TabletOperationId;
import org.apache.accumulo.core.metadata.schema.Ample;
import org.apache.accumulo.core.metadata.schema.DataFileValue;
import org.apache.accumulo.core.metadata.schema.ExternalCompactionId;
@@ -50,7 +49,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Ta
import org.apache.accumulo.core.metadata.schema.MetadataTime;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.LocationType;
-import org.apache.accumulo.core.metadata.schema.TabletOperation;
+import org.apache.accumulo.core.metadata.schema.TabletOperationId;
import org.apache.accumulo.core.tabletserver.log.LogEntry;
import org.apache.accumulo.server.ServerContext;
import org.apache.hadoop.io.Text;
@@ -256,8 +255,8 @@ public abstract class TabletMutatorBase<T extends Ample.TabletUpdates<T>>
}
@Override
- public T putOperation(TabletOperation top, TabletOperationId opId) {
- ServerColumnFamily.OPID_COLUMN.put(mutation, new Value(top.name() + ":" + opId.canonical()));
+ public T putOperation(TabletOperationId opId) {
+ ServerColumnFamily.OPID_COLUMN.put(mutation, new Value(opId.canonical()));
return getThis();
}
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 63d9a7a6a0..c1c3168141 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
@@ -258,4 +258,23 @@ public class MetadataConstraintsTest {
}
+ @Test
+ public void testOperationId() {
+ MetadataConstraints mc = new TestMetadataConstraints();
+ Mutation m;
+ List<Short> violations;
+
+ m = new Mutation(new Text("0;foo"));
+ ServerColumnFamily.OPID_COLUMN.put(m, new Value("bad id"));
+ violations = mc.check(createEnv(), m);
+ assertNotNull(violations);
+ assertEquals(1, violations.size());
+ assertEquals(Short.valueOf((short) 9), violations.get(0));
+
+ m = new Mutation(new Text("0;foo"));
+ ServerColumnFamily.OPID_COLUMN.put(m, new Value("MERGING:FATE[123abc]"));
+ violations = mc.check(createEnv(), m);
+ assertNull(violations);
+ }
+
}
diff --git a/test/src/main/java/org/apache/accumulo/test/functional/AmpleConditionalWriterIT.java b/test/src/main/java/org/apache/accumulo/test/functional/AmpleConditionalWriterIT.java
index 2c897200c4..ed99f59686 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/AmpleConditionalWriterIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/AmpleConditionalWriterIT.java
@@ -38,11 +38,11 @@ import org.apache.accumulo.core.metadata.MetadataTable;
import org.apache.accumulo.core.metadata.RootTable;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.TServerInstance;
-import org.apache.accumulo.core.metadata.TabletOperationId;
import org.apache.accumulo.core.metadata.schema.DataFileValue;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location;
import org.apache.accumulo.core.metadata.schema.TabletMetadata.LocationType;
-import org.apache.accumulo.core.metadata.schema.TabletOperation;
+import org.apache.accumulo.core.metadata.schema.TabletOperationId;
+import org.apache.accumulo.core.metadata.schema.TabletOperationType;
import org.apache.accumulo.core.security.TablePermission;
import org.apache.accumulo.harness.AccumuloClusterHarness;
import org.apache.accumulo.server.metadata.ConditionalTabletsMutatorImpl;
@@ -343,52 +343,46 @@ public class AmpleConditionalWriterIT extends AccumuloClusterHarness {
var context = cluster.getServerContext();
- var opid1 = new TabletOperationId("1234");
- var opid2 = new TabletOperationId("5678");
+ var opid1 = TabletOperationId.from("SPLITTING:FATE[1234]");
+ var opid2 = TabletOperationId.from("MERGING:FATE[5678]");
var ctmi = new ConditionalTabletsMutatorImpl(context);
- ctmi.mutateTablet(e1).requireAbsentOperation().putOperation(TabletOperation.SPLITTING, opid1)
- .submit();
- ctmi.mutateTablet(e2).requireAbsentOperation().putOperation(TabletOperation.MERGING, opid2)
- .submit();
- ctmi.mutateTablet(e3).requireOperation(TabletOperation.SPLITTING, opid1).deleteOperation()
- .submit();
+ ctmi.mutateTablet(e1).requireAbsentOperation().putOperation(opid1).submit();
+ ctmi.mutateTablet(e2).requireAbsentOperation().putOperation(opid2).submit();
+ ctmi.mutateTablet(e3).requireOperation(opid1).deleteOperation().submit();
var results = ctmi.process();
assertEquals(Status.ACCEPTED, results.get(e1).getStatus());
assertEquals(Status.ACCEPTED, results.get(e2).getStatus());
assertEquals(Status.REJECTED, results.get(e3).getStatus());
- assertEquals(TabletOperation.SPLITTING, context.getAmple().readTablet(e1).getOperation());
+ assertEquals(TabletOperationType.SPLITTING,
+ context.getAmple().readTablet(e1).getOperationId().getType());
assertEquals(opid1, context.getAmple().readTablet(e1).getOperationId());
- assertEquals(TabletOperation.MERGING, context.getAmple().readTablet(e2).getOperation());
+ assertEquals(TabletOperationType.MERGING,
+ context.getAmple().readTablet(e2).getOperationId().getType());
assertEquals(opid2, context.getAmple().readTablet(e2).getOperationId());
- assertEquals(null, context.getAmple().readTablet(e3).getOperation());
assertEquals(null, context.getAmple().readTablet(e3).getOperationId());
ctmi = new ConditionalTabletsMutatorImpl(context);
- ctmi.mutateTablet(e1).requireOperation(TabletOperation.MERGING, opid2).deleteOperation()
- .submit();
- ctmi.mutateTablet(e2).requireOperation(TabletOperation.SPLITTING, opid1).deleteOperation()
- .submit();
+ ctmi.mutateTablet(e1).requireOperation(opid2).deleteOperation().submit();
+ ctmi.mutateTablet(e2).requireOperation(opid1).deleteOperation().submit();
results = ctmi.process();
assertEquals(Status.REJECTED, results.get(e1).getStatus());
assertEquals(Status.REJECTED, results.get(e2).getStatus());
- assertEquals(TabletOperation.SPLITTING, context.getAmple().readTablet(e1).getOperation());
- assertEquals(TabletOperation.MERGING, context.getAmple().readTablet(e2).getOperation());
+ assertEquals(TabletOperationType.SPLITTING,
+ context.getAmple().readTablet(e1).getOperationId().getType());
+ assertEquals(TabletOperationType.MERGING,
+ context.getAmple().readTablet(e2).getOperationId().getType());
ctmi = new ConditionalTabletsMutatorImpl(context);
- ctmi.mutateTablet(e1).requireOperation(TabletOperation.SPLITTING, opid1).deleteOperation()
- .submit();
- ctmi.mutateTablet(e2).requireOperation(TabletOperation.MERGING, opid2).deleteOperation()
- .submit();
+ ctmi.mutateTablet(e1).requireOperation(opid1).deleteOperation().submit();
+ ctmi.mutateTablet(e2).requireOperation(opid2).deleteOperation().submit();
results = ctmi.process();
assertEquals(Status.ACCEPTED, results.get(e1).getStatus());
assertEquals(Status.ACCEPTED, results.get(e2).getStatus());
- assertEquals(null, context.getAmple().readTablet(e1).getOperation());
assertEquals(null, context.getAmple().readTablet(e1).getOperationId());
- assertEquals(null, context.getAmple().readTablet(e2).getOperation());
assertEquals(null, context.getAmple().readTablet(e2).getOperationId());
}
}