Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hi Ismael, Sure. Thanks. - Dongjin On Wed, Sep 12, 2018 at 11:56 PM Ismael Juma wrote: > Dongjin, can you please start a vote? > > Ismael > > On Sun, Sep 9, 2018 at 11:15 PM Dongjin Lee wrote: > > > Hi Jason, > > > > You are right. Explicit statements are always better. I updated the > >

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Dongjin, can you please start a vote? Ismael On Sun, Sep 9, 2018 at 11:15 PM Dongjin Lee wrote: > Hi Jason, > > You are right. Explicit statements are always better. I updated the > document following your suggestion. > > @Magnus > > Thanks for the inspection. It seems like a new error code is

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hi Jason, You are right. Explicit statements are always better. I updated the document following your suggestion. @Magnus Thanks for the inspection. It seems like a new error code is not a problem. Thanks, Dongjin On Fri, Sep 7, 2018 at 3:23 AM Jason Gustafson wrote: > Hi Dongjin, > > The

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hi Dongjin, The KIP looks good to me. I'd suggest starting a vote. A couple minor points that might be worth calling out explicitly in the compatibility section: 1. Zstd will only be allowed for the bumped produce API. For older versions, we return UNSUPPORTED_COMPRESSION_TYPE regardless of the

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

> Ismael wrote: > Jason, that's an interesting point regarding the Java client. Do we know > what clients in other languages do in these cases? librdkafka (and its bindings) passes unknown/future errors through to the application, the error code remains intact while the error string will be set

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

I updated the KIP page following the discussion here. Please take a look when you are free. If you have any opinion, don't hesitate to give me a message. Best, Dongjin On Fri, Aug 31, 2018 at 11:35

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

I just updated the draft implementation[^1], rebasing against the latest trunk and implementing error routine (i.e., Error code 74 for UnsupportedCompressionTypeException.) Since we decided to disallow all fetch request below version 2.1.0 for the topics specifying ZStandard, I added an error

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Jason, Great. +1 for UNSUPPORTED_COMPRESSION_TYPE. Best, Dongjin On Thu, Aug 23, 2018 at 8:19 AM Jason Gustafson wrote: > Hey Dongjin, > > Yeah that's right. For what it's worth, librdkafka also appears to handle > unexpected error codes. I expect that most client implementations would >

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hey Dongjin, Yeah that's right. For what it's worth, librdkafka also appears to handle unexpected error codes. I expect that most client implementations would either pass through the raw type or convert to an enum using something like what the java client does. Since we're expecting the client to

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Jason and Ismael, It seems like the only thing we need to regard if we define a new error code (i.e., UNSUPPORTED_COMPRESSION_TYPE) would be the implementation of the other language clients, right? At least, this strategy causes any problem for Java client. Do I understand correctly? Thanks,

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Jason, > I think we would only use this error code when we /know/ that zstd was in use and the client doesn't support it? This is true if either 1) the message needs down-conversion and we encounter a zstd compressed message, or 2) if the topic is explicitly configured to use zstd. Yes, it is

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Jason, that's an interesting point regarding the Java client. Do we know what clients in other languages do in these cases? Ismael On Tue, 21 Aug 2018, 17:30 Jason Gustafson, wrote: > Hi Dongjin, > > One of the complications is that old versions of the API will not expect a > new error code.

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hi Dongjin, One of the complications is that old versions of the API will not expect a new error code. However, since we expect this to be a fatal error anyway for old clients, it may still be more useful to return the correct error code. For example, the Kafka clients use the following code to

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Ismael, Jason and all, I rewrote the backward compatibility strategy & its alternatives like following, based on Ismael & Jason's comments. Since it is not updated to the wiki yet, don't hesitate to give me a message if you have any opinion on it. ``` *Backward Compatibility* We need to

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Sounds reasonable to me. Ismael On Sat, 18 Aug 2018, 12:20 Jason Gustafson, wrote: > Hey Ismael, > > Your summary looks good to me. I think it might also be a good idea to add > a new UNSUPPORTED_COMPRESSION_TYPE error code to go along with the version > bumps. We won't be able to use it for

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hey Ismael, Your summary looks good to me. I think it might also be a good idea to add a new UNSUPPORTED_COMPRESSION_TYPE error code to go along with the version bumps. We won't be able to use it for old api versions since the clients will not understand it, but we can use it going forward so

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hi Dongjin and Jason, I would agree. My summary: 1. Support zstd with message format 2 only. 2. Bump produce and fetch request versions. 3. Provide broker errors whenever possible based on the request version and rely on clients for the cases where the broker can't validate efficiently (example

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hi Dongjin, Yes, that's a good summary. For clients which support v2, the client can parse the message format and hopefully raise a useful error message indicating the unsupported compression type. For older clients, our options are probably (1) to down-convert to the old format using no

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Thanks Jason, I reviewed the down-converting logic following your explanation.[^1] You mean the following routines, right? - https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/KafkaApis.scala#L534 -

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

> > But in my opinion, since the client will fail with the API version, so we > don't need to down-convert the messages anyway. Isn't it? So, I think we > don't care about this case. (I'm sorry, I am not familiar with down-convert > logic.) Currently the broker down-converts automatically when

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Colin and Jason, Thanks for your opinions. In summarizing, the Pros and Cons of bumping fetch API version are: Cons: - The Broker can't know whether a given message batch is compressed with zstd or not. - Need some additional logic for the topic explicitly configured to use zstd. Pros: - The

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hey Colin, The problem for the fetch API is that the broker does not generally know if a batch was compressed with zstd unless it parses it. I think the goal here is to avoid the expensive down-conversion that is needed to ensure compatibility because it is only necessary if zstd is actually in

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Thanks for bumping this, Dongjin. ZStd is a good compression codec and I hope we can get this support in soon! I would say we can just bump the API version to indicate that ZStd support is expected in new clients. We probably need some way of indicating to the older clients that they can't

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

As Kafka 2.0.0 was released, let's reboot this issue, KIP-110 . For newcomers, Here is some summary of the history: KIP-110 was originally worked for the issue KAFKA-4514 but, it lacked benchmark

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Sorry for the late reply. In short, I could not submit the updated KIP by the feature freeze deadline of 2.0.0. For this reason, it will not be included in the 2.0.0 release and all discussion for this issue were postponed after the release of 2.0.0. I have been updating the PR following recent

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

I there any update on this. The performance improvements are quite impressive and I really would like to stop forking kafka just to get this in. Thanks, Bobby On Wed, Jun 13, 2018 at 8:56 PM Dongjin Lee wrote: > Ismael, > > Oh, I forgot all of you are on working frenzy for 2.0! No problem,

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Ismael, Oh, I forgot all of you are on working frenzy for 2.0! No problem, take your time. I am also working at another issue now. Thank you for letting me know. Best, Dongjin On Wed, Jun 13, 2018, 11:44 PM Ismael Juma wrote: > Sorry for the delay Dongjin. Everyone is busy finalising 2.0.0.

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Sorry for the delay Dongjin. Everyone is busy finalising 2.0.0. This KIP seems like a great candidate for 2.1.0 and hopefully there will be more of a discussion next week. :) Ismael On Wed, 13 Jun 2018, 05:17 Dongjin Lee, wrote: > Hello. I just updated my draft implementation: > > 1. Rebased

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hello. I just updated my draft implementation: 1. Rebased to latest trunk (commit 5145d6b) 2. Apply ZStd 1.3.4 You can check out the implementation from here . If you experience any problem running it, don't hesitate to give me a mention. Best, Dongjin

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Here is the short conclusion about the license problem: *We can use zstd and zstd-jni without any problem, but we need to include their license, e.g., BSD license.* Both of BSD 2 Clause License & 3 Clause License requires to include the license used, and BSD 3 Clause License requires that the

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

I greatly appreciate your comprehensive reasoning. so: +1 for b until now. For the license issues, I will have a check on how the over projects are doing and share the results. Best, Dongjin On Mon, Jun 11, 2018 at 10:08 PM Viktor Somogyi wrote: > Hi Dongjin, > > A couple of comments: > I

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hi Dongjin, A couple of comments: I would vote for option b. in the "backward compatibility" section. My reasoning for this is that users upgrading to a zstd compatible version won't start to use it automatically, so manual reconfiguration is required. Therefore an upgrade won't mess up the

Re: [DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Hello, This is Ivan and I still very much support the fact that zstd compression should be included out of the box. Please think about the environment, you can save quite a lot of hardware with it. Thank you. On Sat, Jun 9, 2018 at 14:14 Dongjin Lee wrote: > Since there are no responses for

[DISCUSS] KIP-110: Add Codec for ZStandard Compression (Updated)

Since there are no responses for a week, I decided to reinitiate the discussion thread. https://cwiki.apache.org/confluence/display/KAFKA/KIP-110%3A+Add+Codec+for+ZStandard+Compression This KIP is about to introduce ZStandard Compression into Apache Kafka. The reason why it is posted again has a