Hey Raman, Thanks for the KIP! I think it makes sense. I agree that this becomes especially useful in the context of KIP-939 because transactions can last an indefinite amount of time, but it is useful even today. A large cluster may have a very large number of ongoing transactions at any time, so providing a better way to filter by time will make the tools much more efficient for this common use case.
I have just a couple small comments. 1. In `ListTransactionsOptions`, we use a long in the setter for the duration filter. Can we use `Duration` instead? 2. I think we need to expose `TransactionLastUpdateTimeMs` on `TransactionDescription` as well, right? Thanks, Jason On Tue, Nov 28, 2023 at 8:34 AM Kirk True <k...@kirktrue.pro> wrote: > Hi Raman, > > Thanks for the KIP! > > Questions/comments: > > 1. Is there a Jira created for this? The Jira link still points to > KAFKA-1. > 2. There's a minor typo in the ListTransactionsRequest documentation for > the DurationFilter: trsanactions. > 3. Is the TransactionStartTimeMs return value in the > DescribeTransactionsResponse nullable? > 4. The API uses the general terminology of a "duration filter" but the > CLI uses the specific phrase "running longer than ms." These are both > referring to the same value, right? Can the naming of these two be more > consistent? > 5. What happens when a user runs the updated kafka-transactions.sh script > (using the new argument) against an older broker that doesn't support the > new filter? Does the user get an error, a warning, or a silent ignoring of > the filter? > > Thanks, > Kirk > > On Wed, Nov 15, 2023, at 3:03 PM, Raman Verma wrote: > > Thanks Artem, > > > > I have made changes to the `Public Interfaces` and `Compatibility...` > > sections to incorporate your comment. > > > > On Mon, Nov 6, 2023 at 3:44 PM Raman Verma <raman.x.ve...@gmail.com> > wrote: > > > > > I would like to start a discussion on KIP-994 > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-994%3A+Minor+Enhancements+to+ListTransactions+and+DescribeTransactions+APIs > > > > > > > > >
