[GitHub] [hbase] virajjasani commented on a change in pull request #2345: HBASE-24974: Provide a flexibility to print only row key and filter for multiple tables in the WALPrettyPrinter

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

virajjasani commented on a change in pull request #2345:
URL: https://github.com/apache/hbase/pull/2345#discussion_r482856361



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALPrettyPrinter.java
##########
@@ -274,7 +289,8 @@ public void processFile(final Configuration conf, final Path p)
         Map<String, Object> txn = key.toStringMap();
         long writeTime = key.getWriteTime();
         // check output filters
-        if (table != null && !((TableName) txn.get("table")).toString().equals(table)) {
+        if (!tableList.isEmpty() &&
+          !tableList.contains(((TableName) txn.get("table")).toString())) {

Review comment:
       Looks like this efficient search with `contains()` is the reason behind using `Set` instead of `List`.
   Btw, `(TableName)` is redundant cast here.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALPrettyPrinter.java
##########
@@ -147,11 +153,13 @@ public void setSequenceFilter(long sequence) {
   }
 
   /**
-   * Sets the table filter. Only log entries for this table are printed.
-   * @param table table name to set.
+   * Sets the tables filter. Only log entries for these table are printed.
+   * @param tableListWithDelimiter table names separated with comma.
    */
-  public void setTableFilter(String table) {
-    this.table = table;
+  public void setTableFilter(String tableListWithDelimiter) {
+    for (String table :  tableListWithDelimiter.split(",")) {
+      tableList.add(table);
+    }

Review comment:
       This can be replaced with:
   ```
       Collections.addAll(tableList, tableListWithDelimiter.split(","));
   ```

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALPrettyPrinter.java
##########
@@ -79,9 +81,12 @@
   private boolean outputJSON;
   // The following enable filtering by sequence, region, and row, respectively
   private long sequence;
-  private String table;
+
+  // List of tables for filter
+  private Set<String> tableList;

Review comment:
       +1, and this should be `final` too.




----------------------------------------------------------------
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.

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