You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by mm...@apache.org on 2019/03/12 23:18:05 UTC

[accumulo] branch master updated: Fix lgtm dead logic alerts (#1025)

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 e124fcd  Fix lgtm dead logic alerts (#1025)
e124fcd is described below

commit e124fcd34da80d8d7eae866af6c3b359d56ef075
Author: Mike Miller <mm...@apache.org>
AuthorDate: Tue Mar 12 19:17:59 2019 -0400

    Fix lgtm dead logic alerts (#1025)
    
    * Remove checks that will never be true
    * Add comments for false positives
---
 .../java/org/apache/accumulo/fate/ZooStore.java    |  6 ++--
 .../org/apache/accumulo/tserver/tablet/Tablet.java | 34 +++++-----------------
 2 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/fate/ZooStore.java b/core/src/main/java/org/apache/accumulo/fate/ZooStore.java
index c1064ba..bb6c9f7 100644
--- a/core/src/main/java/org/apache/accumulo/fate/ZooStore.java
+++ b/core/src/main/java/org/apache/accumulo/fate/ZooStore.java
@@ -187,7 +187,8 @@ public class ZooStore<T> implements TStore<T> {
         }
 
         synchronized (this) {
-          if (events == statusChangeEvents) {
+          // suppress lgtm alert - synchronized variable is not always true
+          if (events == statusChangeEvents) { // lgtm [java/constant-comparison]
             if (defered.size() > 0) {
               Long minTime = Collections.min(defered.values());
               long waitTime = minTime - System.currentTimeMillis();
@@ -376,7 +377,8 @@ public class ZooStore<T> implements TStore<T> {
         return status;
 
       synchronized (this) {
-        if (events == statusChangeEvents) {
+        // suppress lgtm alert - synchronized variable is not always true
+        if (events == statusChangeEvents) { // lgtm [java/constant-comparison]
           try {
             this.wait(5000);
           } catch (InterruptedException e) {
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 5cc6af2..d1d78a8 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
@@ -2610,39 +2610,21 @@ public class Tablet {
             return !releaseLock;
         }
 
-        int numAdded = 0;
-        int numContained = 0;
+        boolean added;
+        boolean contained;
         if (addToOther) {
-          if (otherLogs.add(more))
-            numAdded++;
-
-          if (currentLogs.contains(more))
-            numContained++;
+          added = otherLogs.add(more);
+          contained = currentLogs.contains(more);
         } else {
-          if (currentLogs.add(more))
-            numAdded++;
-
-          if (otherLogs.contains(more))
-            numContained++;
+          added = currentLogs.add(more);
+          contained = otherLogs.contains(more);
         }
 
-        if (numAdded > 0) {
+        if (added) {
           rebuildReferencedLogs();
         }
 
-        if (numAdded > 1) {
-          // expect to add all or none
-          throw new IllegalArgumentException(
-              "Added subset of logs " + extent + " " + more + " " + currentLogs);
-        }
-
-        if (numContained > 1) {
-          // expect to contain all or none
-          throw new IllegalArgumentException(
-              "Other logs contained subset of logs " + extent + " " + more + " " + otherLogs);
-        }
-
-        if (numAdded > 0 && numContained == 0) {
+        if (added && !contained) {
           releaseLock = false;
         }