Hi Jun, Good catch! The new configuration is indeed compression.zstd.level instead of compression.snappy.level. I've updated the KIP.
Thanks, Mickael On Wed, Feb 21, 2024 at 7:38 PM Jun Rao <j...@confluent.io.invalid> wrote: > > 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>* > > > > > >
