Ok -- removed Public Interfaces discussion. It should be up to date w/ PR review comments now.
-Dana On Fri, May 6, 2016 at 2:15 PM, Ismael Juma <ism...@juma.me.uk> wrote: > 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 >>
