hi Calvin

Thanks for updating the KIP. there are 2 small questions.

chia_2:

Should `transactionalIdPatternFilter` be renamed to 
filterTransactionalIdPattern for consistency?

chia_3:

the re2j used by consumer is nullable, and the null means nothing. Should we 
align with it?

Best,
Chia-Ping

On 2025/04/04 16:31:08 Calvin Liu wrote:
> Hi Chia-Ping and David,
> Thanks for the advice! It makes sense to use regex for a more general
> filtering purpose. Will update the KIP.
> 
> 
> On Fri, Apr 4, 2025 at 4:31 AM David Jacot <dja...@confluent.io.invalid>
> wrote:
> 
> > Hi Calvin,
> >
> > Thanks for the KIP. It is indeed a nice addition to simplify operations. I
> > have a few comments:
> >
> > DJ01: The motivation is a bit weak in my opinion. It would be great if we
> > could clearly state what you want to do and the rationale behind it. For
> > instance, this sentence "If the transactional ID is well-formatted" is too
> > vague. What does "well-formatted" mean? In your case, you are basically
> > talking about having resources using a common prefix. This is indeed pretty
> > common in Apache Kafka deployments but this is not enforced. Hence, it
> > won't work for everybody.
> >
> > DJ02: I believe that using regular expressions would be a more general way
> > to solve the problem and it would solve for every kafka deployment. Have
> > you considered it? We could reuse re2j for this purpose.
> >
> > Best,
> > David
> >
> > On Fri, Apr 4, 2025 at 12:44 PM Chia-Ping Tsai <chia7...@gmail.com> wrote:
> >
> > > hi Calvin
> > >
> > > thanks for filing this KIP. Some questions are listed below. PTAL
> > >
> > > chia_0:
> > >
> > > could you please add discussion link to the KIP?
> > >
> > > chia_1:
> > >
> > > Have you consider using regex or multi-prefixes (similar to
> > > ProducerIdFilters) instead of prefix? it seems to be more flexible.
> > >
> > > Best,
> > > Chia-Ping
> > >
> > >
> > > Calvin Liu <ca...@confluent.io.invalid> 於 2025年4月4日 週五 上午10:22寫道:
> > >
> > > > Hi Jun
> > > > Good catch, the typo is corrected.
> > > > Thanks!
> > > >
> > > > On Thu, Apr 3, 2025 at 4:17 PM Jun Rao <j...@confluent.io.invalid>
> > wrote:
> > > >
> > > > > Hi, Calvin,
> > > > >
> > > > > Thanks for the KIP. Just a minor comment.
> > > > >
> > > > > ListTransactionRequest no longer supports zkBroker.
> > > > >
> > > > > Jun
> > > > >
> > > > > On Wed, Apr 2, 2025 at 9:43 AM Calvin Liu <ca...@confluent.io.invalid
> > >
> > > > > wrote:
> > > > >
> > > > > > Hi Artem,
> > > > > > Thanks for the advice. It makes sense to make it only fail when
> > using
> > > > the
> > > > > > new filter.
> > > > > > Thanks
> > > > > >
> > > > > > On Tue, Apr 1, 2025 at 4:27 PM Artem Livshits
> > > > > > <alivsh...@confluent.io.invalid> wrote:
> > > > > >
> > > > > > > Hi Calvin,
> > > > > > >
> > > > > > > Thanks for the KIP.
> > > > > > >
> > > > > > > [AL1]
> > > > > > > > When the cluster does not support the new version, the admin
> > > client
> > > > > > will
> > > > > > > fail to build the ListTransaction request with version 2.
> > > > > > > UnsupportedVersionException will be thrown.
> > > > > > >
> > > > > > > It looks like this will make it impossible to use newer clients
> > > with
> > > > > > older
> > > > > > > brokers, which would break backward compatibility.  Instead of
> > > this,
> > > > we
> > > > > > > should do what
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-994%3A+Minor+Enhancements+to+ListTransactions+and+DescribeTransactions+APIs
> > > > > > > did, when it added a new filter: the client should only fail when
> > > the
> > > > > new
> > > > > > > filter is used and the broker doesn't support the new filter.
> > > > > > >
> > > > > > > -Artem
> > > > > > >
> > > > > > > On Tue, Apr 1, 2025 at 4:18 PM Calvin Liu
> > > <ca...@confluent.io.invalid
> > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Kafka Community,
> > > > > > > > I would like to start a discussion on KIP-1152: Add
> > transactional
> > > > ID
> > > > > > > prefix
> > > > > > > > filter to ListTransactions API
> > > > > > > > The main motivation behind the KIP is to reduce the
> > transactions
> > > > > > included
> > > > > > > > in the ListTransactions response when the transactional IDs are
> > > > > > > > well-formatted and can be fetched with a prefix.
> > > > > > > >
> > > > > > > > The KIP will update the ListTransaction APIs,
> > > > ListTransactionOptions
> > > > > > and
> > > > > > > > kafka-transactions.sh
> > > > > > > > Thanks!
> > > > > > > >
> > > > > > > > Link to the KIP.
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1152%3A+Add+transactional+ID+prefix+filter+to+ListTransactions+API
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> 

Reply via email to