You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2018/08/13 14:33:37 UTC
[GitHub] mikewalch closed pull request #594: #582 - Pass ServerContext to
MetadataConstraints
mikewalch closed pull request #594: #582 - Pass ServerContext to MetadataConstraints
URL: https://github.com/apache/accumulo/pull/594
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 253ca1986a..ce9eb3eb86 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
@@ -23,6 +23,7 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
+import java.util.Objects;
import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.constraints.Constraint;
@@ -107,6 +108,7 @@ private static boolean isValidColumn(ColumnUpdate cu) {
@Override
public List<Short> check(Environment env, Mutation mutation) {
+ final ServerContext context = ((SystemEnvironment)env).getServerContext();
ArrayList<Short> violations = null;
@@ -230,7 +232,7 @@ private static boolean isValidColumn(ColumnUpdate cu) {
try {
if (otherTidCount > 0 || !dataFiles.equals(loadedFiles)
- || !getArbitrator().transactionAlive(Constants.BULK_ARBITRATOR_TYPE, tid)) {
+ || !getArbitrator(context).transactionAlive(Constants.BULK_ARBITRATOR_TYPE, tid)) {
violations = addViolation(violations, 8);
}
} catch (Exception ex) {
@@ -264,7 +266,7 @@ private static boolean isValidColumn(ColumnUpdate cu) {
}
if (zooRoot == null) {
- zooRoot = ServerContext.getInstance().getZooKeeperRoot();
+ zooRoot = context.getZooKeeperRoot();
}
boolean lockHeld = false;
@@ -295,8 +297,9 @@ private static boolean isValidColumn(ColumnUpdate cu) {
return violations;
}
- protected Arbitrator getArbitrator() {
- return new ZooArbitrator(ServerContext.getInstance());
+ protected Arbitrator getArbitrator(ServerContext context) {
+ Objects.nonNull(context);
+ return new ZooArbitrator(context);
}
@Override
@@ -327,5 +330,4 @@ protected void finalize() {
if (zooCache != null)
zooCache.clear();
}
-
}
diff --git a/server/base/src/main/java/org/apache/accumulo/server/constraints/SystemEnvironment.java b/server/base/src/main/java/org/apache/accumulo/server/constraints/SystemEnvironment.java
new file mode 100644
index 0000000000..185d5550cf
--- /dev/null
+++ b/server/base/src/main/java/org/apache/accumulo/server/constraints/SystemEnvironment.java
@@ -0,0 +1,26 @@
+/*
+ * 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
+ *
+ * http://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 org.apache.accumulo.core.constraints.Constraint;
+import org.apache.accumulo.server.ServerContext;
+
+public interface SystemEnvironment extends Constraint.Environment {
+
+ ServerContext getServerContext();
+
+}
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 6e9733e3c8..9d0586e1d0 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
@@ -40,7 +40,7 @@
static class TestMetadataConstraints extends MetadataConstraints {
@Override
- protected Arbitrator getArbitrator() {
+ protected Arbitrator getArbitrator(ServerContext context) {
return new Arbitrator() {
@Override
@@ -58,17 +58,23 @@ public boolean transactionComplete(String type, long tid) throws Exception {
}
}
+ private SystemEnvironment createEnv() {
+ SystemEnvironment env = EasyMock.createMock(SystemEnvironment.class);
+ ServerContext context = EasyMock.createMock(ServerContext.class);
+ EasyMock.expect(env.getServerContext()).andReturn(context);
+ EasyMock.replay(env);
+ return env;
+ }
+
@Test
public void testCheck() {
Logger.getLogger(AccumuloConfiguration.class).setLevel(Level.ERROR);
Mutation m = new Mutation(new Text("0;foo"));
TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("1foo".getBytes()));
- ServerContext context = EasyMock.createMock(ServerContext.class);
-
MetadataConstraints mc = new MetadataConstraints();
- List<Short> violations = mc.check(null, m);
+ List<Short> violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
@@ -77,7 +83,7 @@ public void testCheck() {
m = new Mutation(new Text("0:foo"));
TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("1poo".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
@@ -86,7 +92,7 @@ public void testCheck() {
m = new Mutation(new Text("0;foo"));
m.put(new Text("bad_column_name"), new Text(""), new Value("e".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
@@ -95,7 +101,7 @@ public void testCheck() {
m = new Mutation(new Text("!!<"));
TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("1poo".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(2, violations.size());
@@ -105,7 +111,7 @@ public void testCheck() {
m = new Mutation(new Text("0;foo"));
TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
@@ -114,21 +120,21 @@ public void testCheck() {
m = new Mutation(new Text("0;foo"));
TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("bar".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
m = new Mutation(new Text("!0<"));
TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("bar".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
m = new Mutation(new Text("!1<"));
TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN.put(m, new Value("bar".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
@@ -148,7 +154,7 @@ public void testBulkFileCheck() {
new Value("12345".getBytes()));
m.put(DataFileColumnFamily.NAME, new Text("/someFile"),
new DataFileValue(1, 1).encodeAsValue());
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 8), violations.get(0));
@@ -159,7 +165,7 @@ public void testBulkFileCheck() {
new Value("9".getBytes()));
m.put(DataFileColumnFamily.NAME, new Text("/someFile"),
new DataFileValue(1, 1).encodeAsValue());
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 8), violations.get(0));
@@ -170,14 +176,14 @@ public void testBulkFileCheck() {
new Value("5".getBytes()));
m.put(DataFileColumnFamily.NAME, new Text("/someFile"),
new DataFileValue(1, 1).encodeAsValue());
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
// active txid w/o file
m = new Mutation(new Text("0;foo"));
m.put(TabletsSection.BulkFileColumnFamily.NAME, new Text("/someFile"),
new Value("5".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 8), violations.get(0));
@@ -192,7 +198,7 @@ public void testBulkFileCheck() {
new Value("7".getBytes()));
m.put(DataFileColumnFamily.NAME, new Text("/someFile2"),
new DataFileValue(1, 1).encodeAsValue());
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 8), violations.get(0));
@@ -207,7 +213,7 @@ public void testBulkFileCheck() {
new Value("5".getBytes()));
m.put(DataFileColumnFamily.NAME, new Text("/someFile2"),
new DataFileValue(1, 1).encodeAsValue());
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
// two loaded w/ one active txid and one file
@@ -218,7 +224,7 @@ public void testBulkFileCheck() {
new DataFileValue(1, 1).encodeAsValue());
m.put(TabletsSection.BulkFileColumnFamily.NAME, new Text("/someFile2"),
new Value("5".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNotNull(violations);
assertEquals(1, violations.size());
assertEquals(Short.valueOf((short) 8), violations.get(0));
@@ -228,7 +234,7 @@ public void testBulkFileCheck() {
m.put(TabletsSection.BulkFileColumnFamily.NAME, new Text("/someFile"),
new Value("5".getBytes()));
TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value("/t1".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
// inactive txid, mutation that looks like split
@@ -236,7 +242,7 @@ public void testBulkFileCheck() {
m.put(TabletsSection.BulkFileColumnFamily.NAME, new Text("/someFile"),
new Value("12345".getBytes()));
TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value("/t1".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
// active txid, mutation that looks like a load
@@ -245,7 +251,7 @@ public void testBulkFileCheck() {
new Value("5".getBytes()));
m.put(TabletsSection.CurrentLocationColumnFamily.NAME, new Text("789"),
new Value("127.0.0.1:9997".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
// inactive txid, mutation that looks like a load
@@ -254,13 +260,13 @@ public void testBulkFileCheck() {
new Value("12345".getBytes()));
m.put(TabletsSection.CurrentLocationColumnFamily.NAME, new Text("789"),
new Value("127.0.0.1:9997".getBytes()));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
// deleting a load flag
m = new Mutation(new Text("0;foo"));
m.putDelete(TabletsSection.BulkFileColumnFamily.NAME, new Text("/someFile"));
- violations = mc.check(null, m);
+ violations = mc.check(createEnv(), m);
assertNull(violations);
}
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index 9d9b3a1bae..0451a8629d 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -902,7 +902,7 @@ public long startUpdate(TInfo tinfo, TCredentials credentials, TDurability tdura
if (updateMetrics.isEnabled())
updateMetrics.add(TabletServerUpdateMetrics.PERMISSION_ERRORS, 0);
- UpdateSession us = new UpdateSession(new TservConstraintEnv(security, credentials),
+ UpdateSession us = new UpdateSession(new TservConstraintEnv(context, security, credentials),
credentials, durability);
return sessionManager.createSession(us, false);
}
@@ -1268,8 +1268,8 @@ public void update(TInfo tinfo, TCredentials credentials, TKeyExtent tkeyExtent,
final Span prep = Trace.start("prep");
CommitSession cs;
try {
- cs = tablet.prepareMutationsForCommit(new TservConstraintEnv(security, credentials),
- mutations);
+ cs = tablet.prepareMutationsForCommit(
+ new TservConstraintEnv(context, security, credentials), mutations);
} finally {
prep.stop();
}
@@ -1379,7 +1379,7 @@ private void writeConditionalMutations(Map<KeyExtent,List<ServerConditionalMutat
if (mutations.size() > 0) {
CommitSession cs = tablet.prepareMutationsForCommit(
- new TservConstraintEnv(security, sess.credentials), mutations);
+ new TservConstraintEnv(context, security, sess.credentials), mutations);
if (cs == null) {
for (ServerConditionalMutation scm : entry.getValue())
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java
index 3e655186c9..7f068c951e 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TservConstraintEnv.java
@@ -20,22 +20,25 @@
import java.util.Collections;
import org.apache.accumulo.core.client.impl.thrift.ThriftSecurityException;
-import org.apache.accumulo.core.constraints.Constraint.Environment;
import org.apache.accumulo.core.data.ByteSequence;
import org.apache.accumulo.core.data.impl.KeyExtent;
import org.apache.accumulo.core.security.AuthorizationContainer;
import org.apache.accumulo.core.security.Authorizations;
import org.apache.accumulo.core.security.thrift.TCredentials;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.constraints.SystemEnvironment;
import org.apache.accumulo.server.security.SecurityOperation;
-public class TservConstraintEnv implements Environment {
+public class TservConstraintEnv implements SystemEnvironment {
+ private final ServerContext context;
private final TCredentials credentials;
private final SecurityOperation security;
private Authorizations auths;
private KeyExtent ke;
- TservConstraintEnv(SecurityOperation secOp, TCredentials credentials) {
+ TservConstraintEnv(ServerContext context, SecurityOperation secOp, TCredentials credentials) {
+ this.context = context;
this.security = secOp;
this.credentials = credentials;
}
@@ -80,4 +83,9 @@ public boolean contains(ByteSequence auth) {
}
};
}
+
+ @Override
+ public ServerContext getServerContext() {
+ return context;
+ }
}
diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java
index e84e415bad..527b231bb4 100644
--- a/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java
+++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/TservConstraintEnvTest.java
@@ -50,8 +50,8 @@ public void testGetAuthorizationsContainer() throws ThriftSecurityException {
replay(security);
assertTrue(
- new TservConstraintEnv(security, goodCred).getAuthorizationsContainer().contains(bs));
+ new TservConstraintEnv(null, security, goodCred).getAuthorizationsContainer().contains(bs));
assertFalse(
- new TservConstraintEnv(security, badCred).getAuthorizationsContainer().contains(bs));
+ new TservConstraintEnv(null, security, badCred).getAuthorizationsContainer().contains(bs));
}
}
diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ConstraintIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ConstraintIT.java
index 4d67fe719d..a142f855c0 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/ConstraintIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/ConstraintIT.java
@@ -51,7 +51,7 @@
@Override
protected int defaultTimeoutSeconds() {
- return 30;
+ return 60;
}
@Test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services