accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubb...@apache.org
Subject [accumulo] branch 2.0 updated: Remove unused code
Date Tue, 04 Jun 2019 23:47:21 GMT
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 <ctubbsii@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;


Mime
View raw message