[accumulo] branch master updated: Replace try/finally with try-with-resources. (#1546)

2020-03-05 Thread ctubbsii
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
 new 1dcab22  Replace try/finally with try-with-resources. (#1546)
1dcab22 is described below

commit 1dcab224dff2ed38ce5875c36a6a4689d75d5b7c
Author: Christopher Tubbs 
AuthorDate: Thu Mar 5 11:22:56 2020 -0500

Replace try/finally with try-with-resources. (#1546)

Implement the try-with-resources construct available in Java 7+ where 
applicable.
---
 .../java/org/apache/accumulo/core/cli/ClientOpts.java|  8 +---
 .../accumulo/core/clientImpl/InstanceOperationsImpl.java | 12 +++-
 .../core/clientImpl/TabletServerBatchDeleter.java|  7 +--
 .../java/org/apache/accumulo/core/file/rfile/RFile.java  |  5 +
 .../accumulo/core/file/rfile/bcfile/PrintInfo.java   | 10 ++
 .../apache/accumulo/core/util/cleaner/CleanerUtil.java   |  4 +---
 .../hadoop/its/mapred/AccumuloRowInputFormatIT.java  |  8 +---
 .../hadoop/its/mapreduce/AccumuloRowInputFormatIT.java   |  8 +---
 .../miniclusterImpl/MiniAccumuloClusterImpl.java |  8 +---
 .../apache/accumulo/server/master/LiveTServerSet.java|  7 +--
 .../test/ThriftServerBindsBeforeZooKeeperLockIT.java | 16 ++--
 .../test/mapreduce/AccumuloRowInputFormatIT.java |  8 +---
 12 files changed, 16 insertions(+), 85 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 
b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
index ce20f77..338da79 100644
--- a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
+++ b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
@@ -85,16 +85,10 @@ public class ClientOpts extends Help {
 justification = "app is run in same security context as user 
providing the filename")
 @Override
 String process(String value) {
-  Scanner scanner = null;
-  try {
-scanner = new Scanner(new File(value), UTF_8);
+  try (Scanner scanner = new Scanner(new File(value), UTF_8)) {
 return scanner.nextLine();
   } catch (IOException e) {
 throw new ParameterException(e);
-  } finally {
-if (scanner != null) {
-  scanner.close();
-}
   }
 }
   },
diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
index 28e7811..ac3713b 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
@@ -40,7 +40,6 @@ import org.apache.accumulo.core.client.admin.ActiveScan;
 import org.apache.accumulo.core.client.admin.InstanceOperations;
 import org.apache.accumulo.core.clientImpl.thrift.ConfigurationType;
 import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
-import org.apache.accumulo.core.tabletserver.thrift.TabletClientService;
 import org.apache.accumulo.core.tabletserver.thrift.TabletClientService.Client;
 import org.apache.accumulo.core.trace.TraceUtil;
 import org.apache.accumulo.core.util.AddressUtil;
@@ -188,17 +187,12 @@ public class InstanceOperationsImpl implements 
InstanceOperations {
 
   @Override
   public void ping(String tserver) throws AccumuloException {
-TTransport transport = null;
-try {
-  transport = createTransport(AddressUtil.parseAddress(tserver, false), 
context);
-  var client = createClient(new TabletClientService.Client.Factory(), 
transport);
+try (
+TTransport transport = 
createTransport(AddressUtil.parseAddress(tserver, false), context)) {
+  var client = createClient(new Client.Factory(), transport);
   client.getTabletServerStatus(TraceUtil.traceInfo(), context.rpcCreds());
 } catch (TException e) {
   throw new AccumuloException(e);
-} finally {
-  if (transport != null) {
-transport.close();
-  }
 }
   }
 
diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchDeleter.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchDeleter.java
index d644cf6..37606e8 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchDeleter.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchDeleter.java
@@ -52,9 +52,7 @@ public class TabletServerBatchDeleter extends 
TabletServerBatchReader implements
 
   @Override
   public void delete() throws MutationsRejectedException {
-BatchWriter bw = null;
-try {
-  bw = new BatchWriterImpl(context, tableId, bwConfig);
+try (BatchWriter bw = new 

[accumulo] branch master updated: Remove miniDFS from HalfDeadTServerIT (#1545)

2020-03-05 Thread ctubbsii
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
 new 5eda683  Remove miniDFS from HalfDeadTServerIT (#1545)
5eda683 is described below

commit 5eda6838f0127129cd67ccd9f76d124a7cc30091
Author: Christopher Tubbs 
AuthorDate: Thu Mar 5 11:18:09 2020 -0500

Remove miniDFS from HalfDeadTServerIT (#1545)

Remove use of miniDFS from IT, because sometimes miniDFS is slow to
start up, and it causes the IT to time out. It is not needed for the
test, since the test case works fine on LocalFileSystem-based DFS.

Use java.util.scanner to clean up boilerplate BufferedReader stuffs.

Suppress some spammy ZK logs in IT log4j2 config.

Add prefix to DumpOutput class in HalfDeadTServerIT to help troubleshoot
the IT in future.

Build shared library for fake_disk_failure.so only once, before the
class.

Use ProcessBuilder to connect the IO when building the shared library,
so any errors from the compiler are shown on the IT's console capture.

Remove compilation warning about unused variable in fake_disk_failure.c

Add comments and make some variables a bit more explicit.
---
 .../test/functional/HalfDeadTServerIT.java | 142 -
 test/src/main/resources/log4j2-test.properties |   6 +-
 test/src/test/c/fake_disk_failure.c|   1 -
 3 files changed, 84 insertions(+), 65 deletions(-)

diff --git 
a/test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java 
b/test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
index 20abf04..28ec5fa 100644
--- 
a/test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
+++ 
b/test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
@@ -22,16 +22,18 @@ import static 
org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InputStreamReader;
+import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Map;
+import java.util.Scanner;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.accumulo.core.client.Accumulo;
 import org.apache.accumulo.core.client.AccumuloClient;
@@ -44,19 +46,36 @@ import org.apache.accumulo.test.TestIngest;
 import org.apache.accumulo.test.VerifyIngest;
 import org.apache.accumulo.tserver.TabletServer;
 import org.apache.hadoop.conf.Configuration;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
+/**
+ * This test verifies tserver behavior when the node experiences certain 
failure states that cause
+ * it to be unresponsive to network I/O and/or disk access.
+ *
+ * 
+ * This failure state is simulated in a tserver by running that tserver with a 
shared library that
+ * hooks the read/write system calls. When the shared library sees a specific 
trigger file, it
+ * pauses the system calls and prints "sleeping", until that file is deleted, 
after which it proxies
+ * the system call to the real system implementation.
+ *
+ * 
+ * In response to failures of the type this test simulates, the tserver should 
recover if the system
+ * call is stalled for less than the ZooKeeper timeout. Otherwise, it should 
lose its lock in
+ * ZooKeeper and terminate itself.
+ */
 public class HalfDeadTServerIT extends ConfigurableMacBase {
 
   @Override
   public void configure(MiniAccumuloConfigImpl cfg, Configuration 
hadoopCoreSite) {
+// configure only one tserver from mini; mini won't less us configure 0, 
so instead, we will
+// start only 1, and kill it to start our own in the desired simulation 
environment
 cfg.setNumTservers(1);
 cfg.setProperty(Property.INSTANCE_ZK_TIMEOUT, "15s");
 cfg.setProperty(Property.GENERAL_RPC_TIMEOUT, "5s");
 cfg.setProperty(Property.TSERV_NATIVEMAP_ENABLED, 
Boolean.FALSE.toString());
-cfg.useMiniDFS(true);
   }
 
   @Override
@@ -64,41 +83,63 @@ public class HalfDeadTServerIT extends ConfigurableMacBase {
 return 4 * 60;
   }
 
-  class DumpOutput extends Daemon {
+  private static final AtomicBoolean sharedLibBuilt = new AtomicBoolean(false);
 
-private final BufferedReader rdr;
-private final StringBuilder output;
+  @SuppressFBWarnings(value = "COMMAND_INJECTION",
+  justification = "command executed is not from user input")
+  @BeforeClass
+  public static void buildSharedLib() throws IOException, 

[accumulo] branch master updated: Drop obscuring overrides in thrift client code (#1547)

2020-03-05 Thread mmiller
This is an automated email from the ASF dual-hosted git repository.

mmiller pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
 new 39391e9  Drop obscuring overrides in thrift client code (#1547)
39391e9 is described below

commit 39391e9de6815fbcb609c29bd59741c949bb3ac0
Author: Mike Miller 
AuthorDate: Thu Mar 5 08:37:57 2020 -0500

Drop obscuring overrides in thrift client code (#1547)

* Inline method overrides only called once or twice. They obscure actual
method calls and bloat the code
---
 .../accumulo/core/clientImpl/ServerClient.java | 29 --
 .../core/clientImpl/TableOperationsImpl.java   |  3 ++-
 .../core/clientImpl/ThriftTransportPool.java   |  5 +---
 3 files changed, 8 insertions(+), 29 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ServerClient.java 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ServerClient.java
index a19f2f0..197ebad 100644
--- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ServerClient.java
+++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ServerClient.java
@@ -90,7 +90,7 @@ public class ServerClient {
   CT client = null;
   String server = null;
   try {
-Pair pair = ServerClient.getConnection(context, factory);
+Pair pair = ServerClient.getConnection(context, factory, 
true);
 server = pair.getFirst();
 client = pair.getSecond();
 return exec.execute(client);
@@ -112,7 +112,8 @@ public class ServerClient {
   ClientService.Client client = null;
   String server = null;
   try {
-Pair pair = ServerClient.getConnection(context);
+Pair pair =
+ServerClient.getConnection(context, new 
ClientService.Client.Factory(), true);
 server = pair.getFirst();
 client = pair.getSecond();
 exec.execute(client);
@@ -131,31 +132,11 @@ public class ServerClient {
 
   static volatile boolean warnedAboutTServersBeingDown = false;
 
-  public static Pair getConnection(ClientContext 
context)
-  throws TTransportException {
-return getConnection(context, true);
-  }
-
-  public static  Pair 
getConnection(ClientContext context,
-  TServiceClientFactory factory) throws TTransportException {
-return getConnection(context, factory, true, 
context.getClientTimeoutInMillis());
-  }
-
-  public static Pair getConnection(ClientContext 
context,
-  boolean preferCachedConnections) throws TTransportException {
-return getConnection(context, preferCachedConnections, 
context.getClientTimeoutInMillis());
-  }
-
-  public static Pair getConnection(ClientContext 
context,
-  boolean preferCachedConnections, long rpcTimeout) throws 
TTransportException {
-return getConnection(context, new ClientService.Client.Factory(), 
preferCachedConnections,
-rpcTimeout);
-  }
-
   public static  Pair 
getConnection(ClientContext context,
-  TServiceClientFactory factory, boolean preferCachedConnections, long 
rpcTimeout)
+  TServiceClientFactory factory, boolean preferCachedConnections)
   throws TTransportException {
 checkArgument(context != null, "context is null");
+long rpcTimeout = context.getClientTimeoutInMillis();
 // create list of servers
 ArrayList servers = new ArrayList<>();
 
diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
index b83883e..13cffb6 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
@@ -86,6 +86,7 @@ import 
org.apache.accumulo.core.client.summary.SummarizerConfiguration;
 import org.apache.accumulo.core.client.summary.Summary;
 import org.apache.accumulo.core.clientImpl.TabletLocator.TabletLocation;
 import org.apache.accumulo.core.clientImpl.bulk.BulkImport;
+import org.apache.accumulo.core.clientImpl.thrift.ClientService;
 import org.apache.accumulo.core.clientImpl.thrift.ClientService.Client;
 import org.apache.accumulo.core.clientImpl.thrift.TDiskUsage;
 import 
org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
@@ -1431,7 +1432,7 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
 // this operation may us a lot of memory... its likely that 
connections to tabletservers
 // hosting metadata tablets will be cached, so do not use cached
 // connections
-pair = ServerClient.getConnection(context, false);
+pair = ServerClient.getConnection(context, new 
ClientService.Client.Factory(), false);
 diskUsages = pair.getSecond().getDiskUsage(tableNames, 
context.rpcCreds());
   } catch (ThriftTableOperationException e) {