accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubb...@apache.org
Subject accumulo git commit: ACCUMULO-3643 Fix bugs from increasing maxRank for findbugs
Date Sat, 07 Mar 2015 01:48:46 GMT
Repository: accumulo
Updated Branches:
  refs/heads/master a864c6054 -> c939dac61


ACCUMULO-3643 Fix bugs from increasing maxRank for findbugs

Items at rank 7:
Address possible HTTP response splitting in OperationServlet
Correctly generate non-negative random Long numbers in MultiTableRecoveryIT

Items at rank 8:
Remove confusing synchronization on concurrent map
Fix logic of property type regex checks
Check for null in RowFilterTest use of firstKey
Use CountDownLatch in ZooReader instead of synchronization on AtomicBoolean
Make equals checks more strict for tokens (ACCUMULO-3644)
Check return value from putIfAbsent in TCredentialsUpdatingInvocationHandler
Check for null in several places to avoid null pointer dereferencing
Exclude false positive infinite loop in main method of stress test

Items at rank 9:
Exclude thrift-generated code for replication in core


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/c939dac6
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/c939dac6
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/c939dac6

Branch: refs/heads/master
Commit: c939dac61a48ff857565687c6da611558353d884
Parents: a864c60
Author: Christopher Tubbs <ctubbsii@apache.org>
Authored: Wed Mar 4 20:27:27 2015 -0500
Committer: Christopher Tubbs <ctubbsii@apache.org>
Committed: Fri Mar 6 20:47:49 2015 -0500

----------------------------------------------------------------------
 core/src/main/findbugs/exclude-filter.xml       |  3 ++
 .../client/impl/MultiTableBatchWriterImpl.java  | 13 ++---
 .../client/security/tokens/DelegationToken.java | 16 ++----
 .../client/security/tokens/PasswordToken.java   | 13 ++---
 .../apache/accumulo/core/conf/PropertyType.java |  2 +-
 .../core/iterators/user/RowFilterTest.java      |  2 +-
 .../accumulo/fate/zookeeper/ZooReader.java      | 14 ++----
 pom.xml                                         |  2 +-
 .../TCredentialsUpdatingInvocationHandler.java  |  5 +-
 .../security/delegation/AuthenticationKey.java  | 11 +---
 .../server/watcher/Log4jConfiguration.java      |  5 +-
 .../java/org/apache/accumulo/master/Master.java | 24 +++++----
 .../monitor/servlets/OperationServlet.java      | 53 +++++++++++++-------
 .../apache/accumulo/tserver/scan/ScanTask.java  | 30 ++++++++---
 .../apache/accumulo/tserver/tablet/Tablet.java  |  8 ++-
 test/pom.xml                                    |  7 +++
 test/src/main/findbugs/exclude-filter.xml       | 23 +++++++++
 .../server/security/SystemCredentialsIT.java    |  2 +
 .../test/MasterRepairsDualAssignmentIT.java     |  2 +-
 .../accumulo/test/MultiTableRecoveryIT.java     |  3 +-
 20 files changed, 145 insertions(+), 93 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/core/src/main/findbugs/exclude-filter.xml
----------------------------------------------------------------------
diff --git a/core/src/main/findbugs/exclude-filter.xml b/core/src/main/findbugs/exclude-filter.xml
index 88b7922..138260c 100644
--- a/core/src/main/findbugs/exclude-filter.xml
+++ b/core/src/main/findbugs/exclude-filter.xml
@@ -33,4 +33,7 @@
     <Match>
         <Class name="~org\.apache\.accumulo\.core\.tabletserver\.thrift\..*" />
     </Match>
+    <Match>
+        <Class name="~org\.apache\.accumulo\.core\.replication\.thrift\..*" />
+    </Match>
 </FindBugsFilter>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
b/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
index 82aa714..ea18d07 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
@@ -220,12 +220,13 @@ public class MultiTableBatchWriterImpl implements MultiTableBatchWriter
{
 
     String tableId = getId(tableName);
 
-    synchronized (tableWriters) {
-      BatchWriter tbw = tableWriters.get(tableId);
-      if (tbw == null) {
-        tbw = new TableBatchWriter(tableId);
-        tableWriters.put(tableId, tbw);
-      }
+    BatchWriter tbw = tableWriters.get(tableId);
+    if (tbw == null) {
+      tbw = new TableBatchWriter(tableId);
+      BatchWriter current = tableWriters.putIfAbsent(tableId, tbw);
+      // return the current one if another thread created one first
+      return current != null ? current : tbw;
+    } else {
       return tbw;
     }
   }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/core/src/main/java/org/apache/accumulo/core/client/security/tokens/DelegationToken.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/DelegationToken.java
b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/DelegationToken.java
index cc4864a..4f8a107 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/DelegationToken.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/DelegationToken.java
@@ -21,7 +21,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.Set;
 
@@ -143,19 +142,12 @@ public class DelegationToken extends PasswordToken {
     return super.hashCode() ^ identifier.hashCode();
   }
 
+  /*
+   * We assume we can cast obj to DelegationToken because the super.equals(obj) check ensures
obj is of the same type as this
+   */
   @Override
   public boolean equals(Object obj) {
-    if (this == obj)
-      return true;
-    if (obj == null)
-      return false;
-    if (!(obj instanceof DelegationToken))
-      return false;
-    DelegationToken other = (DelegationToken) obj;
-    if (!Arrays.equals(getPassword(), other.getPassword())) {
-      return false;
-    }
-    return identifier.equals(other.identifier);
+    return super.equals(obj) && identifier.equals(((DelegationToken) obj).identifier);
   }
 
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
index eb1abe6..e11c3b0 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
@@ -103,16 +103,13 @@ public class PasswordToken implements AuthenticationToken {
     return Arrays.hashCode(password);
   }
 
+  /*
+   * Instances of PasswordToken should only be considered equal if they are of the same type.
This check is done here to ensure that this class is equal to the
+   * class of the object being checked.
+   */
   @Override
   public boolean equals(Object obj) {
-    if (this == obj)
-      return true;
-    if (obj == null)
-      return false;
-    if (!(obj instanceof PasswordToken))
-      return false;
-    PasswordToken other = (PasswordToken) obj;
-    return Arrays.equals(password, other.password);
+    return this == obj || (obj != null && getClass().equals(obj.getClass()) &&
Arrays.equals(password, ((PasswordToken) obj).password));
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
index 09055d5..a100513 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
@@ -105,6 +105,6 @@ public enum PropertyType {
    * @return true if value is valid or null, or if this type has no regex
    */
   public boolean isValidFormat(String value) {
-    return (regex == null && value == null) ? true : regex.matcher(value).matches();
+    return (regex == null || value == null) ? true : regex.matcher(value).matches();
   }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/core/src/test/java/org/apache/accumulo/core/iterators/user/RowFilterTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/iterators/user/RowFilterTest.java
b/core/src/test/java/org/apache/accumulo/core/iterators/user/RowFilterTest.java
index 461f609..7914ec0 100644
--- a/core/src/test/java/org/apache/accumulo/core/iterators/user/RowFilterTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/iterators/user/RowFilterTest.java
@@ -80,7 +80,7 @@ public class RowFilterTest {
         rowIterator.next();
       }
 
-      rowIterator.seek(new Range(firstKey.getRow(), false, null, true), new HashSet<ByteSequence>(),
false);
+      rowIterator.seek(new Range(firstKey == null ? null : firstKey.getRow(), false, null,
true), new HashSet<ByteSequence>(), false);
       while (rowIterator.hasTop()) {
         sum2 += Integer.parseInt(rowIterator.getTopValue().toString());
         rowIterator.next();

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReader.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReader.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReader.java
index 707959c..cec54f5 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReader.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReader.java
@@ -17,7 +17,7 @@
 package org.apache.accumulo.fate.zookeeper;
 
 import java.util.List;
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.accumulo.fate.zookeeper.ZooUtil.ZooKeeperConnectionInfo;
@@ -221,21 +221,15 @@ public class ZooReader implements IZooReader {
   @Override
   public void sync(final String path) throws KeeperException, InterruptedException {
     final AtomicInteger rc = new AtomicInteger();
-    final AtomicBoolean waiter = new AtomicBoolean(false);
+    final CountDownLatch waiter = new CountDownLatch(1);
     getZooKeeper().sync(path, new VoidCallback() {
       @Override
       public void processResult(int code, String arg1, Object arg2) {
         rc.set(code);
-        synchronized (waiter) {
-          waiter.set(true);
-          waiter.notifyAll();
-        }
+        waiter.countDown();
       }
     }, null);
-    synchronized (waiter) {
-      while (!waiter.get())
-        waiter.wait();
-    }
+    waiter.await();
     Code code = Code.get(rc.get());
     if (code != KeeperException.Code.OK) {
       throw KeeperException.create(code);

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index d8c6d02..776aff5 100644
--- a/pom.xml
+++ b/pom.xml
@@ -528,7 +528,7 @@
             <effort>Max</effort>
             <failOnError>true</failOnError>
             <includeTests>true</includeTests>
-            <maxRank>6</maxRank>
+            <maxRank>10</maxRank>
           </configuration>
         </plugin>
         <plugin>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/server/base/src/main/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandler.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandler.java
b/server/base/src/main/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandler.java
index 150f0d3..4e0e931 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandler.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandler.java
@@ -144,8 +144,9 @@ public class TCredentialsUpdatingInvocationHandler<I> implements
InvocationHandl
       }
       typedClz = clz.asSubclass(AuthenticationToken.class);
     }
-    TOKEN_CLASS_CACHE.putIfAbsent(tokenClassName, typedClz);
-    return typedClz;
+    // return the current one and throw away the one we just created if some other thread
created it first
+    Class<? extends AuthenticationToken> current = TOKEN_CLASS_CACHE.putIfAbsent(tokenClassName,
typedClz);
+    return current != null ? current : typedClz;
   }
 
   private Object invokeMethod(Method method, Object[] args) throws Throwable {

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationKey.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationKey.java
b/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationKey.java
index 134502a..35fd115 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationKey.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationKey.java
@@ -24,6 +24,7 @@ import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.util.Objects;
 
 import javax.crypto.SecretKey;
 
@@ -98,15 +99,7 @@ public class AuthenticationKey implements Writable {
 
   @Override
   public boolean equals(Object obj) {
-    if (obj == null || !(obj instanceof AuthenticationKey)) {
-      return false;
-    }
-    AuthenticationKey other = (AuthenticationKey) obj;
-    // authKey might be null due to writable nature
-    if (null == authKey && null != other.authKey) {
-      return false;
-    }
-    return authKey.equals(other.authKey);
+    return obj != null && obj instanceof AuthenticationKey && Objects.equals(authKey,
((AuthenticationKey) obj).authKey);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/server/base/src/main/java/org/apache/accumulo/server/watcher/Log4jConfiguration.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/watcher/Log4jConfiguration.java
b/server/base/src/main/java/org/apache/accumulo/server/watcher/Log4jConfiguration.java
index 7ff5542..b9e6805 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/watcher/Log4jConfiguration.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/watcher/Log4jConfiguration.java
@@ -22,6 +22,8 @@ import org.apache.log4j.LogManager;
 import org.apache.log4j.PropertyConfigurator;
 import org.apache.log4j.xml.DOMConfigurator;
 
+import com.google.common.base.Preconditions;
+
 /**
  * Encapsulate calls to PropertyConfigurator or DOMConfigurator to set up logging
  */
@@ -33,7 +35,8 @@ public class Log4jConfiguration {
   private final String auditConfig;
 
   public Log4jConfiguration(String filename) {
-    usingProperties = (filename != null && filename.endsWith(".properties"));
+    Preconditions.checkNotNull(filename, "log4j configuration filename must not be null");
+    usingProperties = filename.endsWith(".properties");
     this.filename = filename;
     log4jFile = new File(filename);
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/server/master/src/main/java/org/apache/accumulo/master/Master.java
----------------------------------------------------------------------
diff --git a/server/master/src/main/java/org/apache/accumulo/master/Master.java b/server/master/src/main/java/org/apache/accumulo/master/Master.java
index 5e6dcfb..219144a 100644
--- a/server/master/src/main/java/org/apache/accumulo/master/Master.java
+++ b/server/master/src/main/java/org/apache/accumulo/master/Master.java
@@ -859,14 +859,14 @@ public class Master extends AccumuloServerContext implements LiveTServerSet.List
     private boolean goodStats() {
       int start;
       switch (getMasterState()) {
-      case UNLOAD_METADATA_TABLETS:
-        start = 1;
-        break;
-      case UNLOAD_ROOT_TABLET:
-        start = 2;
-        break;
-      default:
-        start = 0;
+        case UNLOAD_METADATA_TABLETS:
+          start = 1;
+          break;
+        case UNLOAD_ROOT_TABLET:
+          start = 2;
+          break;
+        default:
+          start = 0;
       }
       for (int i = start; i < watchers.size(); i++) {
         TabletGroupWatcher watcher = watchers.get(i);
@@ -947,7 +947,7 @@ public class Master extends AccumuloServerContext implements LiveTServerSet.List
                   break;
               }
           }
-        }catch(Throwable t) {
+        } catch (Throwable t) {
           log.error("Error occurred reading / switching master goal state. Will continue
with attempt to update status", t);
         }
 
@@ -1231,8 +1231,10 @@ public class Master extends AccumuloServerContext implements LiveTServerSet.List
     replicationWorkDriver.join(remaining(deadline));
     replAddress.server.stop();
     // Signal that we want it to stop, and wait for it to do so.
-    authenticationTokenKeyManager.gracefulStop();
-    authenticationTokenKeyManager.join(remaining(deadline));
+    if (authenticationTokenKeyManager != null) {
+      authenticationTokenKeyManager.gracefulStop();
+      authenticationTokenKeyManager.join(remaining(deadline));
+    }
 
     // quit, even if the tablet servers somehow jam up and the watchers
     // don't stop

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/OperationServlet.java
----------------------------------------------------------------------
diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/OperationServlet.java
b/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/OperationServlet.java
index 41149e4..a3dbe8c 100644
--- a/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/OperationServlet.java
+++ b/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/OperationServlet.java
@@ -17,7 +17,10 @@
 package org.apache.accumulo.monitor.servlets;
 
 import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
 
+import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -42,6 +45,7 @@ public class OperationServlet extends BasicServlet {
   @Override
   public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
     String redir = null;
+    List<Cookie> cookiesToSet = Collections.emptyList();
     try {
       String operation = req.getParameter("action");
       redir = req.getParameter("redir");
@@ -57,7 +61,7 @@ public class OperationServlet extends BasicServlet {
           if (t instanceof WebOperation) {
             WebOperation op = (WebOperation) t;
             if (op.getClass().getSimpleName().equalsIgnoreCase(operation + "Operation"))
{
-              op.execute(req, resp, log);
+              cookiesToSet = op.execute(req, log);
               break;
             }
           }
@@ -67,9 +71,10 @@ public class OperationServlet extends BasicServlet {
       log.error(t, t);
     } finally {
       try {
-        redir = redir == null ? "" : redir;
-        redir = redir.startsWith("/") ? redir.substring(1) : redir;
-        resp.sendRedirect("/" + redir); // this makes findbugs happy
+        for (Cookie c : cookiesToSet) {
+          resp.addCookie(c);
+        }
+        resp.sendRedirect(sanitizeRedirect(redir));
         resp.flushBuffer();
       } catch (Throwable t) {
         log.error(t, t);
@@ -77,40 +82,50 @@ public class OperationServlet extends BasicServlet {
     }
   }
 
+  private static String sanitizeRedirect(final String url) {
+    if (url == null || url.isEmpty() || url.contains("\r") || url.contains("\n")) {
+      // prevent HTTP response splitting
+      return "/";
+    }
+    return url.startsWith("/") ? url : ("/" + url);
+  }
+
   private interface WebOperation {
-    void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) throws Exception;
+    List<Cookie> execute(HttpServletRequest req, Logger log) throws Exception;
   }
 
   public static class RefreshOperation implements WebOperation {
     @Override
-    public void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) {
+    public List<Cookie> execute(HttpServletRequest req, Logger log) {
       String value = req.getParameter("value");
-      BasicServlet.setCookie(resp, "page.refresh.rate", value == null ? "5" : BasicServlet.encode(value));
+      return Collections.singletonList(new Cookie("page.refresh.rate", value == null ? "5"
: BasicServlet.encode(value)));
     }
   }
 
   public static class ClearLogOperation implements WebOperation {
     @Override
-    public void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) {
+    public List<Cookie> execute(HttpServletRequest req, Logger log) {
       LogService.getInstance().clear();
+      return Collections.emptyList();
     }
   }
 
   public static class ClearTableProblemsOperation implements WebOperation {
     @Override
-    public void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) {
+    public List<Cookie> execute(HttpServletRequest req, Logger log) {
       String table = req.getParameter("table");
       try {
         ProblemReports.getInstance(Monitor.getContext()).deleteProblemReports(table);
       } catch (Exception e) {
         log.error("Failed to delete problem reports for table " + table, e);
       }
+      return Collections.emptyList();
     }
   }
 
   public static class ClearProblemOperation implements WebOperation {
     @Override
-    public void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) {
+    public List<Cookie> execute(HttpServletRequest req, Logger log) {
       String table = req.getParameter("table");
       String resource = req.getParameter("resource");
       String ptype = req.getParameter("ptype");
@@ -119,52 +134,54 @@ public class OperationServlet extends BasicServlet {
       } catch (Exception e) {
         log.error("Failed to delete problem reports for table " + table, e);
       }
+      return Collections.emptyList();
     }
   }
 
   public static class SortTableOperation implements WebOperation {
     @Override
-    public void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) throws
IOException {
+    public List<Cookie> execute(HttpServletRequest req, Logger log) throws IOException
{
       String page = req.getParameter("page");
       String table = req.getParameter("table");
       String asc = req.getParameter("asc");
       String col = req.getParameter("col");
       if (table == null || page == null || (asc == null && col == null))
-        return;
+        return Collections.emptyList();
       page = BasicServlet.encode(page);
       table = BasicServlet.encode(table);
       if (asc == null) {
         col = BasicServlet.encode(col);
-        BasicServlet.setCookie(resp, "tableSort." + page + "." + table + "." + "sortCol",
col);
+        return Collections.singletonList(new Cookie("tableSort." + page + "." + table + "."
+ "sortCol", col));
       } else {
         asc = BasicServlet.encode(asc);
-        BasicServlet.setCookie(resp, "tableSort." + page + "." + table + "." + "sortAsc",
asc);
+        return Collections.singletonList(new Cookie("tableSort." + page + "." + table + "."
+ "sortAsc", asc));
       }
     }
   }
 
   public static class ToggleLegendOperation implements WebOperation {
     @Override
-    public void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) throws
Exception {
+    public List<Cookie> execute(HttpServletRequest req, Logger log) throws Exception
{
       String page = req.getParameter("page");
       String table = req.getParameter("table");
       String show = req.getParameter("show");
       if (table == null || page == null || show == null)
-        return;
+        return Collections.emptyList();
       page = BasicServlet.encode(page);
       table = BasicServlet.encode(table);
       show = BasicServlet.encode(show);
-      BasicServlet.setCookie(resp, "tableLegend." + page + "." + table + "." + "show", show);
+      return Collections.singletonList(new Cookie("tableLegend." + page + "." + table + "."
+ "show", show));
     }
   }
 
   public static class ClearDeadServerOperation implements WebOperation {
     @Override
-    public void execute(HttpServletRequest req, HttpServletResponse resp, Logger log) {
+    public List<Cookie> execute(HttpServletRequest req, Logger log) {
       String server = req.getParameter("server");
       // a dead server should have a uniq address: a logger or tserver
       DeadServerList obit = new DeadServerList(ZooUtil.getRoot(Monitor.getContext().getInstance())
+ Constants.ZDEADTSERVERS);
       obit.delete(server);
+      return Collections.emptyList();
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java
----------------------------------------------------------------------
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java
index dffdf76..abf3d1a 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java
@@ -77,22 +77,38 @@ public abstract class ScanTask<T> implements RunnableFuture<T>
{
     throw new UnsupportedOperationException();
   }
 
-  @SuppressWarnings("unchecked")
   @Override
   public T get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException,
TimeoutException {
 
     ArrayBlockingQueue<Object> localRQ = resultQueue;
 
-    if (state.get() == CANCELED)
+    if (isCancelled())
       throw new CancellationException();
 
-    if (localRQ == null && state.get() == ADDED)
-      throw new IllegalStateException("Tried to get result twice");
+    if (localRQ == null) {
+      int st = state.get();
+      String stateStr;
+      switch (st) {
+        case ADDED:
+          stateStr = "ADDED";
+          break;
+        case CANCELED:
+          stateStr = "CANCELED";
+          break;
+        case INITIAL:
+          stateStr = "INITIAL";
+          break;
+        default:
+          stateStr = "UNKONWN";
+          break;
+      }
+      throw new IllegalStateException("Tried to get result twice [state=" + stateStr + "("
+ st + ")]");
+    }
 
     Object r = localRQ.poll(timeout, unit);
 
     // could have been canceled while waiting
-    if (state.get() == CANCELED) {
+    if (isCancelled()) {
       if (r != null)
         throw new IllegalStateException("Nothing should have been added when in canceled
state");
 
@@ -109,7 +125,9 @@ public abstract class ScanTask<T> implements RunnableFuture<T>
{
     if (r instanceof Throwable)
       throw new ExecutionException((Throwable) r);
 
-    return (T) r;
+    @SuppressWarnings("unchecked")
+    T rAsT = (T) r;
+    return rAsT;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
----------------------------------------------------------------------
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
index f2d5375..edcf2ff 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
@@ -146,6 +146,7 @@ import org.apache.zookeeper.KeeperException.NoNodeException;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
 
 /**
  *
@@ -461,10 +462,7 @@ public class Tablet implements TabletCommitter {
     if (null == tblConf) {
       Tables.clearCache(tabletServer.getInstance());
       tblConf = tabletServer.getTableConfiguration(extent);
-      if (null == tblConf) {
-        // Not guaranteed to be non-null, but should be. A failed load will be re-assigned
though..
-        log.warn("Could not get table configuration for " + extent.getTableId().toString());
-      }
+      Preconditions.checkNotNull(tblConf, "Could not get table configuration for " + extent.getTableId().toString());
     }
 
     this.tableConfiguration = tblConf;
@@ -1757,7 +1755,7 @@ public class Tablet implements TabletCommitter {
         Key first = pair.getFirst();
         Key last = pair.getSecond();
         // If first and last are null, it's an empty file. Add it to the compact set so it
goes away.
-        if ((first == null && last == null) || !extent.contains(first.getRow()) ||
!extent.contains(last.getRow())) {
+        if ((first == null && last == null) || (first != null && !extent.contains(first.getRow()))
|| (last != null && !extent.contains(last.getRow()))) {
           result.add(file);
         }
       }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/test/pom.xml
----------------------------------------------------------------------
diff --git a/test/pom.xml b/test/pom.xml
index 5862688..a7ce39d 100644
--- a/test/pom.xml
+++ b/test/pom.xml
@@ -230,6 +230,13 @@
             </systemPropertyVariables>
           </configuration>
         </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>findbugs-maven-plugin</artifactId>
+          <configuration>
+            <excludeFilterFile>src/main/findbugs/exclude-filter.xml</excludeFilterFile>
+          </configuration>
+        </plugin>
       </plugins>
     </pluginManagement>
   </build>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/test/src/main/findbugs/exclude-filter.xml
----------------------------------------------------------------------
diff --git a/test/src/main/findbugs/exclude-filter.xml b/test/src/main/findbugs/exclude-filter.xml
new file mode 100644
index 0000000..ea6b83e
--- /dev/null
+++ b/test/src/main/findbugs/exclude-filter.xml
@@ -0,0 +1,23 @@
+<!--
+  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.
+-->
+<FindBugsFilter>
+    <Match>
+        <Class name="org.apache.accumulo.test.stress.random.Write" />
+        <Method name="main" params="java.lang.String[]" returns="void" />
+        <Bug code="IL" pattern="IL_INFINITE_LOOP" />
+    </Match>
+</FindBugsFilter>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java
b/test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java
index 9f00fcb..5e85ddb 100644
--- a/test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java
+++ b/test/src/test/java/org/apache/accumulo/server/security/SystemCredentialsIT.java
@@ -203,6 +203,8 @@ public class SystemCredentialsIT extends ConfigurableMacIT {
 
       };
       creds = new SystemCredentials(inst, "!SYSTEM", new PasswordToken("fake"));
+    } else {
+      throw new RuntimeException("Incorrect usage; expected to be run by test only");
     }
     Instance instance = HdfsZooInstance.getInstance();
     Connector conn;

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/test/src/test/java/org/apache/accumulo/test/MasterRepairsDualAssignmentIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/test/MasterRepairsDualAssignmentIT.java
b/test/src/test/java/org/apache/accumulo/test/MasterRepairsDualAssignmentIT.java
index fb5b4a0..d95e0cd 100644
--- a/test/src/test/java/org/apache/accumulo/test/MasterRepairsDualAssignmentIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/MasterRepairsDualAssignmentIT.java
@@ -107,7 +107,7 @@ public class MasterRepairsDualAssignmentIT extends ConfigurableMacIT {
       for (TabletLocationState tls : store) {
         if (tls != null && tls.current != null) {
           states.add(tls.current);
-        } else if (tls.extent.equals(new KeyExtent(new Text(ReplicationTable.ID), null, null)))
{
+        } else if (tls != null && tls.extent.equals(new KeyExtent(new Text(ReplicationTable.ID),
null, null))) {
           replStates.add(tls.current);
         } else {
           allAssigned = false;

http://git-wip-us.apache.org/repos/asf/accumulo/blob/c939dac6/test/src/test/java/org/apache/accumulo/test/MultiTableRecoveryIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/test/MultiTableRecoveryIT.java b/test/src/test/java/org/apache/accumulo/test/MultiTableRecoveryIT.java
index a241f71..bf2b1ac 100644
--- a/test/src/test/java/org/apache/accumulo/test/MultiTableRecoveryIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/MultiTableRecoveryIT.java
@@ -72,7 +72,8 @@ public class MultiTableRecoveryIT extends ConfigurableMacIT {
     System.out.println("writing");
     final Random random = new Random();
     for (i = 0; i < 1_000_000; i++) {
-      long randomRow = Math.abs(random.nextLong());
+      // make non-negative avoiding Math.abs, because that can still be negative
+      long randomRow = random.nextLong() & Long.MAX_VALUE;
       assertTrue(randomRow >= 0);
       final int table = (int) (randomRow % N);
       final Mutation m = new Mutation(Long.toHexString(randomRow));


Mime
View raw message