You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ct...@apache.org on 2019/06/04 23:47:21 UTC
[accumulo] branch 2.0 updated: Remove unused code
This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch 2.0
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.0 by this push:
new f10b407 Remove unused code
f10b407 is described below
commit f10b4073dba6b8095e9934e9ea158eb9f45c6f67
Author: Christopher Tubbs <ct...@apache.org>
AuthorDate: Tue Jun 4 19:42:58 2019 -0400
Remove unused code
* Remove `@Ignore` JUnit tests
* Remove unnecessary use of PowerMock when EasyMock sufficed
* Remove unnecessary duplicate parameter in Initialize.checkInit
Note: IDE added missing curly braces around single-statement blocks; I
kept these as a side effect, because, it's better to have them and the
formatter can't be configured to automatically add them, so they get
added as I edit.
---
.../apache/accumulo/server/init/Initialize.java | 78 ++++++++++++++--------
.../accumulo/server/init/InitializeTest.java | 28 ++------
.../accumulo/server/tablets/TabletTimeTest.java | 1 -
server/tserver/pom.xml | 5 --
.../apache/accumulo/tserver/InMemoryMapTest.java | 49 --------------
.../accumulo/tserver/TservConstraintEnvTest.java | 4 +-
6 files changed, 56 insertions(+), 109 deletions(-)
diff --git a/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java b/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java
index 519138f..2f855b8 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java
@@ -138,8 +138,9 @@ public class Initialize implements KeywordExecutable {
private static ZooReaderWriter zoo = null;
private static ConsoleReader getConsoleReader() throws IOException {
- if (reader == null)
+ if (reader == null) {
reader = new ConsoleReader();
+ }
return reader;
}
@@ -242,12 +243,13 @@ public class Initialize implements KeywordExecutable {
ReplicationUtil.STATUS_FORMATTER_CLASS_NAME);
}
- static boolean checkInit(Configuration conf, VolumeManager fs, SiteConfiguration sconf,
- Configuration hadoopConf) throws IOException {
+ static boolean checkInit(VolumeManager fs, SiteConfiguration sconf, Configuration hadoopConf)
+ throws IOException {
@SuppressWarnings("deprecation")
String fsUri = sconf.get(Property.INSTANCE_DFS_URI);
- if (fsUri.equals(""))
- fsUri = FileSystem.getDefaultUri(conf).toString();
+ if (fsUri.equals("")) {
+ fsUri = FileSystem.getDefaultUri(hadoopConf).toString();
+ }
log.info("Hadoop Filesystem is {}", fsUri);
log.info("Accumulo data dirs are {}",
Arrays.asList(VolumeConfiguration.getVolumeUris(sconf, hadoopConf)));
@@ -318,7 +320,7 @@ public class Initialize implements KeywordExecutable {
public boolean doInit(SiteConfiguration siteConfig, Opts opts, Configuration conf,
VolumeManager fs) throws IOException {
- if (!checkInit(conf, fs, siteConfig, conf)) {
+ if (!checkInit(fs, siteConfig, conf)) {
return false;
}
@@ -478,8 +480,9 @@ public class Initialize implements KeywordExecutable {
Path iidLocation = new Path(baseDir, ServerConstants.INSTANCE_ID_DIR);
fs.mkdirs(iidLocation);
fs.createNewFile(new Path(iidLocation, uuid.toString()));
- if (print)
+ if (print) {
log.info("Initialized volume {}", baseDir);
+ }
}
}
@@ -607,8 +610,9 @@ public class Initialize implements KeywordExecutable {
NodeExistsPolicy.SKIP, Ids.OPEN_ACL_UNSAFE);
// setup instance name
- if (opts.clearInstanceName)
+ if (opts.clearInstanceName) {
zoo.recursiveDelete(instanceNamePath, NodeMissingPolicy.SKIP);
+ }
zoo.putPersistentData(instanceNamePath, uuid.getBytes(UTF_8), NodeExistsPolicy.FAIL);
final byte[] EMPTY_BYTE_ARRAY = new byte[0];
@@ -685,11 +689,13 @@ public class Initialize implements KeywordExecutable {
} else {
instanceName = opts.cliInstanceName;
}
- if (instanceName == null)
+ if (instanceName == null) {
System.exit(0);
+ }
instanceName = instanceName.trim();
- if (instanceName.length() == 0)
+ if (instanceName.length() == 0) {
continue;
+ }
instanceNamePath = Constants.ZROOT + Constants.ZINSTANCES + "/" + instanceName;
if (opts.clearInstanceName) {
exists = false;
@@ -699,8 +705,9 @@ public class Initialize implements KeywordExecutable {
if (exists) {
String decision = getConsoleReader().readLine("Instance name \"" + instanceName
+ "\" exists. Delete existing entry from zookeeper? [Y/N] : ");
- if (decision == null)
+ if (decision == null) {
System.exit(0);
+ }
if (decision.length() == 1 && decision.toLowerCase(Locale.ENGLISH).charAt(0) == 'y') {
opts.clearInstanceName = true;
exists = false;
@@ -747,14 +754,17 @@ public class Initialize implements KeywordExecutable {
do {
rootpass = getConsoleReader().readLine(
"Enter initial password for " + rootUser + getInitialPasswordWarning(siteConfig), '*');
- if (rootpass == null)
+ if (rootpass == null) {
System.exit(0);
+ }
confirmpass =
getConsoleReader().readLine("Confirm initial password for " + rootUser + ": ", '*');
- if (confirmpass == null)
+ if (confirmpass == null) {
System.exit(0);
- if (!rootpass.equals(confirmpass))
+ }
+ if (!rootpass.equals(confirmpass)) {
log.error("Passwords do not match");
+ }
} while (!rootpass.equals(confirmpass));
return rootpass.getBytes(UTF_8);
}
@@ -772,10 +782,11 @@ public class Initialize implements KeywordExecutable {
private String getInitialPasswordWarning(SiteConfiguration siteConfig) {
String optionalWarning;
Property authenticatorProperty = Property.INSTANCE_SECURITY_AUTHENTICATOR;
- if (siteConfig.get(authenticatorProperty).equals(authenticatorProperty.getDefaultValue()))
+ if (siteConfig.get(authenticatorProperty).equals(authenticatorProperty.getDefaultValue())) {
optionalWarning = ": ";
- else
+ } else {
optionalWarning = " (this may not be applicable for your security setup): ";
+ }
return optionalWarning;
}
@@ -792,30 +803,36 @@ public class Initialize implements KeywordExecutable {
// Hadoop 0.23 switched the min value configuration name
int min = Math.max(hadoopConf.getInt("dfs.replication.min", 1),
hadoopConf.getInt("dfs.namenode.replication.min", 1));
- if (max < 5)
+ if (max < 5) {
setMetadataReplication(max, "max");
- if (min > 5)
+ }
+ if (min > 5) {
setMetadataReplication(min, "min");
+ }
for (Entry<String,String> entry : initialMetadataConf.entrySet()) {
if (!TablePropUtil.setTableProperty(zoo, zooKeeperRoot, RootTable.ID, entry.getKey(),
- entry.getValue()))
+ entry.getValue())) {
throw new IOException("Cannot create per-table property " + entry.getKey());
+ }
if (!TablePropUtil.setTableProperty(zoo, zooKeeperRoot, MetadataTable.ID, entry.getKey(),
- entry.getValue()))
+ entry.getValue())) {
throw new IOException("Cannot create per-table property " + entry.getKey());
+ }
}
// Only add combiner config to accumulo.metadata table (ACCUMULO-3077)
for (Entry<String,String> entry : initialMetadataCombinerConf.entrySet()) {
if (!TablePropUtil.setTableProperty(zoo, zooKeeperRoot, MetadataTable.ID, entry.getKey(),
- entry.getValue()))
+ entry.getValue())) {
throw new IOException("Cannot create per-table property " + entry.getKey());
+ }
}
// add configuration to the replication table
for (Entry<String,String> entry : initialReplicationTableConf.entrySet()) {
if (!TablePropUtil.setTableProperty(zoo, zooKeeperRoot, ReplicationTable.ID, entry.getKey(),
- entry.getValue()))
+ entry.getValue())) {
throw new IOException("Cannot create per-table property " + entry.getKey());
+ }
}
} catch (Exception e) {
log.error("FATAL: Error talking to ZooKeeper", e);
@@ -828,11 +845,12 @@ public class Initialize implements KeywordExecutable {
.readLine("Your HDFS replication " + reason + " is not compatible with our default "
+ MetadataTable.NAME + " replication of 5. What do you want to set your "
+ MetadataTable.NAME + " replication to? (" + replication + ") ");
- if (rep == null || rep.length() == 0)
+ if (rep == null || rep.length() == 0) {
rep = Integer.toString(replication);
- else
+ } else {
// Lets make sure it's a number
Integer.parseInt(rep);
+ }
initialMetadataConf.put(Property.TABLE_FILE_REPLICATION.getKey(), rep);
}
@@ -840,8 +858,9 @@ public class Initialize implements KeywordExecutable {
Configuration hadoopConf) throws IOException {
for (String baseDir : VolumeConfiguration.getVolumeUris(siteConfig, hadoopConf)) {
if (fs.exists(new Path(baseDir, ServerConstants.INSTANCE_ID_DIR))
- || fs.exists(new Path(baseDir, ServerConstants.VERSION_DIR)))
+ || fs.exists(new Path(baseDir, ServerConstants.VERSION_DIR))) {
return true;
+ }
}
return false;
@@ -867,12 +886,13 @@ public class Initialize implements KeywordExecutable {
UUID uuid = UUID.fromString(ZooUtil.getInstanceIDFromHdfs(iidPath, siteConfig, hadoopConf));
for (Pair<Path,Path> replacementVolume : ServerConstants.getVolumeReplacements(siteConfig,
hadoopConf)) {
- if (aBasePath.equals(replacementVolume.getFirst()))
+ if (aBasePath.equals(replacementVolume.getFirst())) {
log.error(
"{} is set to be replaced in {} and should not appear in {}."
+ " It is highly recommended that this property be removed as data"
+ " could still be written to this volume.",
aBasePath, Property.INSTANCE_VOLUMES_REPLACEMENTS, Property.INSTANCE_VOLUMES);
+ }
}
if (ServerUtil.getAccumuloPersistentVersion(versionPath.getFileSystem(hadoopConf), versionPath)
@@ -967,9 +987,11 @@ public class Initialize implements KeywordExecutable {
addVolumes(fs, siteConfig, hadoopConfig);
}
- if (!opts.resetSecurity && !opts.addVolumes)
- if (!doInit(siteConfig, opts, hadoopConfig, fs))
+ if (!opts.resetSecurity && !opts.addVolumes) {
+ if (!doInit(siteConfig, opts, hadoopConfig, fs)) {
System.exit(-1);
+ }
+ }
} catch (Exception e) {
log.error("Fatal exception", e);
throw new RuntimeException(e);
diff --git a/server/base/src/test/java/org/apache/accumulo/server/init/InitializeTest.java b/server/base/src/test/java/org/apache/accumulo/server/init/InitializeTest.java
index f57a394..522df6a 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/init/InitializeTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/init/InitializeTest.java
@@ -34,7 +34,6 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
/**
@@ -94,7 +93,7 @@ public class InitializeTest {
expect(zoo.exists("/")).andReturn(false);
replay(zoo);
- assertFalse(Initialize.checkInit(conf, fs, sconf, conf));
+ assertFalse(Initialize.checkInit(fs, sconf, conf));
}
@SuppressWarnings("deprecation")
@@ -109,26 +108,7 @@ public class InitializeTest {
expect(fs.exists(anyObject(Path.class))).andReturn(true);
replay(fs);
- assertFalse(Initialize.checkInit(conf, fs, sconf, conf));
- }
-
- // Cannot test, need to mock static FileSystem.getDefaultUri()
- @SuppressWarnings("deprecation")
- @Ignore
- @Test
- public void testCheckInit_AlreadyInit_DefaultUri() throws Exception {
- expect(sconf.get(Property.INSTANCE_DFS_URI)).andReturn("").anyTimes();
- expect(sconf.get(Property.INSTANCE_DFS_DIR)).andReturn("/bar");
- expect(sconf.get(Property.INSTANCE_SECRET))
- .andReturn(Property.INSTANCE_SECRET.getDefaultValue());
- replay(sconf);
- expect(zoo.exists("/")).andReturn(true);
- replay(zoo);
- // expect(fs.getUri()).andReturn(new URI("hdfs://default"));
- expect(fs.exists(anyObject(Path.class))).andReturn(true);
- replay(fs);
-
- assertFalse(Initialize.checkInit(conf, fs, sconf, conf));
+ assertFalse(Initialize.checkInit(fs, sconf, conf));
}
@SuppressWarnings("deprecation")
@@ -144,7 +124,7 @@ public class InitializeTest {
expect(fs.exists(anyObject(Path.class))).andThrow(new IOException());
replay(fs);
- Initialize.checkInit(conf, fs, sconf, conf);
+ Initialize.checkInit(fs, sconf, conf);
}
@SuppressWarnings("deprecation")
@@ -160,6 +140,6 @@ public class InitializeTest {
expect(fs.exists(anyObject(Path.class))).andReturn(false);
replay(fs);
- assertTrue(Initialize.checkInit(conf, fs, sconf, conf));
+ assertTrue(Initialize.checkInit(fs, sconf, conf));
}
}
diff --git a/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java b/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java
index e8fff7c..1bbf7c2 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java
@@ -110,7 +110,6 @@ public class TabletTimeTest {
}
@Test
- @org.junit.Ignore
public void testMaxMetadataTime_Null3() {
assertNull(TabletTime.maxMetadataTime(null, null));
}
diff --git a/server/tserver/pom.xml b/server/tserver/pom.xml
index 1f60854..bdfc524 100644
--- a/server/tserver/pom.xml
+++ b/server/tserver/pom.xml
@@ -108,11 +108,6 @@
<scope>test</scope>
</dependency>
<dependency>
- <groupId>org.powermock</groupId>
- <artifactId>powermock-api-easymock</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
<scope>test</scope>
diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/InMemoryMapTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/InMemoryMapTest.java
index d35082f..a904a71 100644
--- a/server/tserver/src/test/java/org/apache/accumulo/tserver/InMemoryMapTest.java
+++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/InMemoryMapTest.java
@@ -27,13 +27,9 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
-import java.util.List;
import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeMap;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.accumulo.core.client.SampleNotPresentException;
@@ -64,13 +60,10 @@ import org.apache.accumulo.tserver.InMemoryMap.MemoryIterator;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.io.Text;
import org.easymock.EasyMock;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.TemporaryFolder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import com.google.common.collect.ImmutableMap;
@@ -503,8 +496,6 @@ public class InMemoryMapTest {
testAndCallNext(skvi1, "r1", "foo:cq", 3, "v1");
}
- private static final Logger log = LoggerFactory.getLogger(InMemoryMapTest.class);
-
static long sum(long[] counts) {
long result = 0;
for (long count : counts) {
@@ -513,46 +504,6 @@ public class InMemoryMapTest {
return result;
}
- // - hard to get this timing test to run well on apache build machines
- @Test
- @Ignore
- public void parallelWriteSpeed() throws Exception {
- List<Double> timings = new ArrayList<>();
- for (int threads : new int[] {1, 2, 16, /* 64, 256 */}) {
- final long now = System.currentTimeMillis();
- final long[] counts = new long[threads];
- final InMemoryMap imm = newInMemoryMap(false, tempFolder.newFolder().getAbsolutePath());
- ExecutorService e = Executors.newFixedThreadPool(threads);
- for (int j = 0; j < threads; j++) {
- final int threadId = j;
- e.execute(() -> {
- while (System.currentTimeMillis() - now < 1000) {
- for (int k = 0; k < 1000; k++) {
- Mutation m = new Mutation("row");
- m.put("cf", "cq", new Value("v".getBytes()));
- List<Mutation> mutations = Collections.singletonList(m);
- imm.mutate(mutations, 1);
- counts[threadId]++;
- }
- }
- });
- }
- e.shutdown();
- e.awaitTermination(10, TimeUnit.SECONDS);
- imm.delete(10000);
- double mutationsPerSecond = sum(counts) / ((System.currentTimeMillis() - now) / 1000.);
- timings.add(mutationsPerSecond);
- log.info("{}",
- String.format("%.1f mutations per second with %d threads", mutationsPerSecond, threads));
- }
- // verify that more threads doesn't go a lot faster, or a lot slower than one thread
- for (Double timing : timings) {
- double ratioFirst = timings.get(0) / timing;
- assertTrue(ratioFirst < 3);
- assertTrue(ratioFirst > 0.3);
- }
- }
-
@Test
public void testLocalityGroups() throws Exception {
ConfigurationCopy config = newConfig(tempFolder.newFolder().getAbsolutePath());
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 dc2130d..c8e48ee 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
@@ -16,11 +16,11 @@
*/
package org.apache.accumulo.tserver;
+import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
-import static org.powermock.api.easymock.PowerMock.createMock;
-import static org.powermock.api.easymock.PowerMock.replay;
import java.nio.ByteBuffer;
import java.util.Collections;