NicoK commented on issue #6705: [FLINK-10356][network] add sanity checks to SpillingAdaptiveSpanningRecordDeserializer URL: https://github.com/apache/flink/pull/6705#issuecomment-440237825 Thanks for the new reviews @zhijiangW and @pnowojski. @zhijiangW: 1. True, if we could do this in `SpanningWrapper` or `NonSpanningWrapper`/its created input view, it would be better to grasp. However, these work on a byte level and are called by the user's de/serializer. We don't know when the user has finished reading (for checking whether too few bytes have been consumed). I could hack some `finishRecordDeserialization()` method in but this would also miss a few things we are adding now: record length and how many bytes have been read after the record length. We could hack these in as well but would also have to wrap any exception in its read methods to throw ours instead (for reading too many bytes). -> I do agree that this whole deserialization code should eventually be simplified, but not as part of this PR. 2. I could actually move the error-checking code up into `SpillingAdaptiveSpanningRecordDeserializer#getNextRecord()` as well. I think, it makes sense to have it together in one place - done.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
