Hi, Mickael, Thanks for the updated KIP.
There is a typo. The KIP says that it adds a new option compression.snappy.level, but later says that Snappy is excluded. Otherwise, the changes look good to me. Jun On Wed, Feb 7, 2024 at 6:40 AM Mickael Maison <mickael.mai...@gmail.com> wrote: > Hi Divij, > > Thanks for bringing that point. After reading KIP-984, I don't think > it supersedes KIP-390/KIP-780. Being able to tune the built-in codecs > would directly benefit many users. It may also cover some scenarios > that motivated KIP-984 without requiring users to write a custom > codec. > I've not commented in the KIP-984 thread yet but at the moment it > seems very light on details (no proposed API for codecs, no > explanations of error scenarios if clients or brokers don't have > compatible codecs), including the motivation which is important when > exposing new APIs. On the other hand, KIP-390/KIP-780 have much more > details with benchmarks to support the motivation. > > In my opinion starting with the compression level (KIP-390) is a good > first step and I think we should focus on that and deliver it. I > believe one of the reasons KIP-780 wasn't voted is because we never > delivered KIP-390 and nobody was keen on building a KIP on top of > another undelivered KIP. > > Thanks, > Mickael > > > On Wed, Feb 7, 2024 at 12:27 PM Divij Vaidya <divijvaidy...@gmail.com> > wrote: > > > > Hey Mickael > > > > Since this KIP was written, we have a new proposal to make the > compression > > completely pluggable > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-984%3A+Add+pluggable+compression+interface+to+Kafka > . > > If we implement that KIP, would it supersede the need for adding fine > grain > > compression controls in Kafka? > > > > It might be beneficial to have a joint proposal of these two KIPs which > may > > satisfy both use cases. > > > > -- > > Divij Vaidya > > > > > > > > On Wed, Feb 7, 2024 at 11:14 AM Mickael Maison <mickael.mai...@gmail.com > > > > wrote: > > > > > Hi, > > > > > > I'm resurrecting this old thread as this KIP would be a nice > > > improvement and almost 3 years later the PR for this KIP has still not > > > been merged! > > > > > > The reason is that during reviews we noticed the proposed > > > configuration, compression.level, was not easy to use as each codec > > > has its own valid range of levels [0]. > > > > > > As proposed by Jun in the PR [1], I updated the KIP to use > > > compression.<codec>.level configurations instead of a single > > > compression.level setting. This syntax would also line up with the > > > proposal to add per-codec configuration options from KIP-780 [2] > > > (still to be voted). I moved the original proposal to the rejected > > > section. > > > > > > I've put the original voters and KIP author on CC. Let me know if you > > > have any feedback. > > > > > > 0: https://github.com/apache/kafka/pull/10826 > > > 1: https://github.com/apache/kafka/pull/10826#issuecomment-1795952612 > > > 2: > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-780%3A+Support+fine-grained+compression+options > > > > > > Thanks, > > > Mickael > > > > > > > > > On Fri, Jun 11, 2021 at 10:00 AM Dongjin Lee <dong...@apache.org> > wrote: > > > > > > > > This KIP is now passed with: > > > > > > > > - binding: +3 (Ismael, Tom, Konstantine) > > > > - non-binding: +1 (Ryanne) > > > > > > > > Thanks again to all the supporters. I also updated the KIP by moving > the > > > > compression buffer option into the 'Future Works' section, as Ismael > > > > proposed. > > > > > > > > Best, > > > > Dongjin > > > > > > > > > > > > > > > > On Fri, Jun 11, 2021 at 3:03 AM Konstantine Karantasis > > > > <konstant...@confluent.io.invalid> wrote: > > > > > > > > > Makes sense. Looks like a good improvement. Thanks for including > the > > > > > evaluation in the proposal Dongjin. > > > > > > > > > > +1 (binding) > > > > > > > > > > Konstantine > > > > > > > > > > On Wed, Jun 9, 2021 at 6:59 PM Dongjin Lee <dong...@apache.org> > wrote: > > > > > > > > > > > Thanks Ismel, Tom and Ryanne, > > > > > > > > > > > > I am now updating the KIP about the further works. Sure, You > won't be > > > > > > disappointed. > > > > > > > > > > > > As of Present: > > > > > > > > > > > > - binding: +2 (Ismael, Tom) > > > > > > - non-binding: +1 (Ryanne) > > > > > > > > > > > > Anyone else? > > > > > > > > > > > > Best, > > > > > > Dongjin > > > > > > > > > > > > On Thu, Jun 10, 2021 at 2:03 AM Tom Bentley <tbent...@redhat.com > > > > > wrote: > > > > > > > > > > > > > Hi Dongjin, > > > > > > > > > > > > > > Thanks for the KIP, +1 (binding). > > > > > > > > > > > > > > Kind regards, > > > > > > > > > > > > > > Tom > > > > > > > > > > > > > > On Wed, Jun 9, 2021 at 5:16 PM Ismael Juma <ism...@juma.me.uk> > > > wrote: > > > > > > > > > > > > > > > I'm +1 on the proposed change. As I stated in the discuss > > > thread, I > > > > > > don't > > > > > > > > think we should rule out the buffer size config, but we could > > > list > > > > > that > > > > > > > as > > > > > > > > future work vs rejected alternatives. > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > > > On Sat, Jun 5, 2021 at 2:37 PM Dongjin Lee < > dong...@apache.org> > > > > > wrote: > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > I'd like to open a voting thread for KIP-390: Support > > > Compression > > > > > > Level > > > > > > > > > (rebooted): > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-390%3A+Support+Compression+Level > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > Dongjin > > > > > > > > > > > > > > > > > > -- > > > > > > > > > *Dongjin Lee* > > > > > > > > > > > > > > > > > > *A hitchhiker in the mathematical world.* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > *github: <http://goog_969573159/>github.com/dongjinleekr > > > > > > > > > <https://github.com/dongjinleekr>keybase: > > > > > > > > https://keybase.io/dongjinleekr > > > > > > > > > <https://keybase.io/dongjinleekr>linkedin: > > > > > > > > kr.linkedin.com/in/dongjinleekr > > > > > > > > > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > > > > > > > > speakerdeck.com/dongjin > > > > > > > > > <https://speakerdeck.com/dongjin>* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > *Dongjin Lee* > > > > > > > > > > > > *A hitchhiker in the mathematical world.* > > > > > > > > > > > > > > > > > > > > > > > > *github: <http://goog_969573159/>github.com/dongjinleekr > > > > > > <https://github.com/dongjinleekr>keybase: > > > > > https://keybase.io/dongjinleekr > > > > > > <https://keybase.io/dongjinleekr>linkedin: > > > > > kr.linkedin.com/in/dongjinleekr > > > > > > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > > > > > speakerdeck.com/dongjin > > > > > > <https://speakerdeck.com/dongjin>* > > > > > > > > > > > > > > > > > > > > > > > -- > > > > *Dongjin Lee* > > > > > > > > *A hitchhiker in the mathematical world.* > > > > > > > > > > > > > > > > *github: <http://goog_969573159/>github.com/dongjinleekr > > > > <https://github.com/dongjinleekr>keybase: > > > https://keybase.io/dongjinleekr > > > > <https://keybase.io/dongjinleekr>linkedin: > > > kr.linkedin.com/in/dongjinleekr > > > > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > > speakerdeck.com/dongjin > > > > <https://speakerdeck.com/dongjin>* > > > >