From commits-return-22937-archive-asf-public=cust-asf.ponee.io@accumulo.apache.org Tue Jun 4 23:47:22 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 59C4018064D for ; Wed, 5 Jun 2019 01:47:22 +0200 (CEST) Received: (qmail 17537 invoked by uid 500); 4 Jun 2019 23:47:21 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 17524 invoked by uid 99); 4 Jun 2019 23:47:21 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 04 Jun 2019 23:47:21 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 93E538AA96; Tue, 4 Jun 2019 23:47:21 +0000 (UTC) Date: Tue, 04 Jun 2019 23:47:21 +0000 To: "commits@accumulo.apache.org" Subject: [accumulo] branch 2.0 updated: Remove unused code MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <155969204147.9830.17309504804448692920@gitbox.apache.org> From: ctubbsii@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: accumulo X-Git-Refname: refs/heads/2.0 X-Git-Reftype: branch X-Git-Oldrev: 4b3887a5f5fc9909f6a56a3fd5afce69b69b0e9e X-Git-Newrev: f10b4073dba6b8095e9934e9ea158eb9f45c6f67 X-Git-Rev: f10b4073dba6b8095e9934e9ea158eb9f45c6f67 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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 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 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 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 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 @@ test - org.powermock - powermock-api-easymock - test - - org.slf4j slf4j-log4j12 test 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 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 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;