Thanks for the review.
A comment mentions a 1-byte txid, I think you meant opid. Having the expected format in the comment is super helpful too, thanks. Mind replicating it for the other two readers too?
Why move the temp array into a member? It'll probably be stack allocated and thus no GC pressure, 4KB should fit too.
Hmm. I'm not sure if it will be stack allocated or not. The compiler would have to do some pretty fancy escape analysis since we're passing the array to many functions, any of which might hold a reference. It seems easier just to keep the buffer around, since we're using it a lot.
Great opportunity to fix the LayoutVersion.EDITS_CHESKUM typo
LengthPrefixedReader#scanOp, shouldn't this be basically readOp but without the second pass to readFields? Not sure why it doesn't validate the max edit size or the checksum. The TODO is relevant.
Yeah, you're right... we should be validating checksums here. Fixed.
It looks like this was actually a regression in edit log validation introduced by
HDFS-6038. Previously, the JN did validate checksums when attempting to skip to a specific op, and after that change, it didn't.
Can we get a unit test for this situation?
Yeah, let me add a unit test that scanOp validates checksums. It should be possible to do as a true unit test too.
Is some code sharing is possible among the three readOp's for getting the FSEditLogOp, this bit?
I'm on the fence about this since I think it might make it harder to read since the control flow would be passing from base->derived->base again.