One more suggestion Dana, I would remove the "Public interfaces" section as those classes are not actually public (only the classes with Javadoc are public: https://kafka.apache.org/090/javadoc/index.html) and the information in the KIP is a bit stale when compared to the PR.
Ismael On Fri, May 6, 2016 at 10:12 PM, Ismael Juma <ism...@juma.me.uk> wrote: > On Sun, May 1, 2016 at 3:57 AM, Dana Powers <dana.pow...@gmail.com> wrote: >> >> > 2. We're completely disabling checksumming of the compressed payload on >> > consumption. Normally you'd want to validate each level of framing for >> > correct end-to-end validation. You could still do this (albeit more >> weakly) >> > by validating the checksum is one of the two potentially valid values >> > (correct checksum or old, incorrect checksum). This obviously has >> > computational cost. Are we sure the tradeoff we're going with makes >> sense? >> >> Yes, to be honest, not validating on consumption is mostly because I just >> haven't dug into the bowels of the java client compressor / memory records >> call chains. It seems non-trivial to switch validation based on the >> message >> version in the consumer code. I did not opt for the weak validation that >> you >> suggest because I think the broker should always validate v1 messages on >> produce, and that piece shares the same code path within the lz4 java >> classes. >> I suppose we could make the default to raise an error on checksums that >> fail >> weak validation, and then switch to strong validation in the broker. >> Alternately, >> if you have suggestions on how to wire up the consumer code to switch lz4 >> behavior based on message version, I would be happy to run with that. > > > The lack of checksum validation on consumption was a concern I had as well > (and Jun too, when I checked with him) so I helped Dana with this and the > PR now includes consumer validation for V1 messages. Dana, can you please > update the KIP? > > Ismael >
