+1 One caveat on the vote though -- I don't know the details of LZ4 (format, libraries, etc) well enough to have a handle on whether the changes under "KafkaLZ4* code" are going to be sufficient to get broad support from other LZ4 libraries. Are we going to have multiple implementations we can test a PR with before we merge? Or would any subsequent fixes also have to come with bumping the produce request version to indicate varying feature support if we had to further change that code? What I want to avoid is clients in some languages having to work with broker version numbers instead of protocol version numbers due to further incompatibilities we might find.
-Ewen On Thu, Apr 28, 2016 at 9:01 AM, Dana Powers <dana.pow...@gmail.com> wrote: > Sure thing. Yes, the substantive change is fixing the HC checksum. > > But to further improve interoperability, the kafka LZ4 class would no > longer reject messages that have these optional header flags set. The > flags might get set if the client/user chooses to use a non-java lz4 > compression library that includes them. In practice, naive support for > the flags just means reading a few extra bytes in the header and/or > footer of the payload. The KIP does not intend to use or validate this > extra data. > > ContentSize is described as: "This field has no impact on decoding, it > just informs the decoder how much data the frame holds (for example, > to display it during decoding process, or for verification purpose). > It can be safely skipped by a conformant decoder." We skip it. > > ContentChecksum is "Content Checksum validates the result, that all > blocks were fully transmitted in the correct order and without error, > and also that the encoding/decoding process itself generated no > distortion." We skip it. > > -Dana > > > On Thu, Apr 28, 2016 at 7:43 AM, Jun Rao <j...@confluent.io> wrote: > > Hi, Dana, > > > > Could you explain the following from the KIP a bit more? The KIP is > > intended to just fix the HC checksum, but the following seems to suggest > > there are other format changes? > > > > KafkaLZ4* code: > > > > - add naive support for optional header flags (ContentSize, > > ContentChecksum) to enable interoperability with off-the-shelf lz4 > libraries > > - the only flag left unsupported is dependent-block compression, which > > our implementation does not currently support. > > > > > > Thanks, > > > > Jun > > > > On Mon, Apr 25, 2016 at 2:26 PM, Dana Powers <dana.pow...@gmail.com> > wrote: > > > >> Hi all, > >> > >> Initiating a vote thread because the KIP-57 proposal is specific to > >> the 0.10 release. > >> > >> KIP-57 can be accessed here: > >> < > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-57+-+Interoperable+LZ4+Framing > >> >. > >> > >> The related JIRA is https://issues.apache.org/jira/browse/KAFKA-3160 > >> and working github PR at https://github.com/apache/kafka/pull/1212 > >> > >> The vote will run for 72 hours. > >> > >> +1 (non-binding) > >> > -- Thanks, Ewen
