[GitHub] [accumulo] EdColeman commented on a diff in pull request #2755: modify filter conditions for status

[GitBox <gi...@apache.org>]

EdColeman commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r890185929


##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
 
       zs.unreserve(tid, 0);
 
-      if ((filterTxid != null && !filterTxid.contains(tid))
-          || (filterStatus != null && !filterStatus.contains(status)))
-        continue;
-
-      statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+      if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+          || filterStatus.contains(status)) {
+        statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+      }

Review Comment:
   For me, yes.  Can you verify by adding the accumulo.properties where you were testing / seeing the issue?
   
   What I don't understand is why the filter seemed to change - needing the properties for a test using fate print is understandable, but the same filter code seems to be behavior seems to differ between the two branch and I don't see why.  That's why I set this to draft until the difference can be explained.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org