chia7712 commented on code in PR #19355:
URL: https://github.com/apache/kafka/pull/19355#discussion_r2045050285


##########
core/src/main/scala/kafka/coordinator/transaction/TransactionStateManager.scala:
##########
@@ -343,6 +345,9 @@ class TransactionStateManager(brokerId: Int,
             false
           } else if (filterDurationMs >= 0 && (now - 
txnMetadata.txnStartTimestamp) <= filterDurationMs) {
             false
+          } else if (!filterTransactionalIdPattern.isEmpty &&

Review Comment:
   `filterTransactionalIdPattern.nonEmpty`



##########
clients/src/main/resources/common/message/ListTransactionsRequest.json:
##########
@@ -30,6 +32,9 @@
     },
     { "name": "DurationFilter", "type": "int64", "versions": "1+", "default": 
-1,
       "about": "Duration (in millis) to filter by: if < 0, all transactions 
will be returned; otherwise, only transactions running longer than this 
duration will be returned."
+    },
+    { "name": "TransactionalIdPatternFilter", "type": "string", "versions": 
"2+", "nullableVersions": "2+", "default": "null",

Review Comment:
   Could you please add version check to 
`ListTransactionsRequest.Builder#build`? 
   ```java
               if (data.transactionalIdPatternFilter() != null && version < 2) {
                   throw new UnsupportedVersionException("xxxx");
               }
   ```



##########
clients/src/main/java/org/apache/kafka/clients/admin/ListTransactionsOptions.java:
##########
@@ -70,6 +71,19 @@ public ListTransactionsOptions filterOnDuration(long 
durationMs) {
         return this;
     }
 
+    /**
+     * Filter only the transactions that match with the given transactional ID 
pattern.
+     * If no filter is specified or if the passed string is empty,

Review Comment:
   The protocol says `if null, all transactions are returned;` Should we align 
it with `null`?
   
   BTW, `ConsumerGroupHeartbeatRequest` uses `null` 



##########
core/src/main/scala/kafka/coordinator/transaction/TransactionStateManager.scala:
##########
@@ -343,6 +345,9 @@ class TransactionStateManager(brokerId: Int,
             false
           } else if (filterDurationMs >= 0 && (now - 
txnMetadata.txnStartTimestamp) <= filterDurationMs) {
             false
+          } else if (!filterTransactionalIdPattern.isEmpty &&
+            
!Pattern.compile(filterTransactionalIdPattern).matcher(txnMetadata.transactionalId).matches())
 {

Review Comment:
   Should we avoid compiling the regex for each transaction id?



-- 
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: jira-unsubscr...@kafka.apache.org

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

Reply via email to