jsancio commented on code in PR #12457:
URL: https://github.com/apache/kafka/pull/12457#discussion_r936987763
##########
raft/src/test/java/org/apache/kafka/raft/internals/RecordsIteratorTest.java:
##########
@@ -85,18 +88,41 @@ public void testFileRecords(
FileRecords fileRecords = FileRecords.open(TestUtils.tempFile());
fileRecords.append(memRecords);
- testIterator(batches, fileRecords);
+ testIterator(batches, fileRecords, true);
+ }
+
+ @Property
+ public void testCrcValidation(
+ @ForAll CompressionType compressionType,
+ @ForAll long seed
+ ) throws IOException {
+ List<TestBatch<String>> batches = createBatches(seed);
+ MemoryRecords memRecords = buildRecords(compressionType, batches);
+ // Corrupt the record buffer
+
memRecords.buffer().putInt(DefaultRecordBatch.LAST_OFFSET_DELTA_OFFSET, new
Random(seed).nextInt());
Review Comment:
Does a random int always corrupt the batch? Meaning, it is possible that
this test sometime fails because it was unlucky to pick a random int that
didn't invalidate the CRC.
Can you explain in a comment why this buffer/memory change specifically?
##########
raft/src/main/java/org/apache/kafka/raft/internals/RecordsIterator.java:
##########
@@ -179,8 +182,13 @@ private Optional<Batch<T>> nextBatch() {
return Optional.empty();
}
- private Batch<T> readBatch(DefaultRecordBatch batch) {
+ private Batch<T> readBatch(DefaultRecordBatch batch) throws
CorruptRecordException {
final Batch<T> result;
+ if (doCrcValidation) {
+ // Perform a CRC validity check on this block.
Review Comment:
typo; did you mean "on this batch."?
##########
raft/src/main/java/org/apache/kafka/raft/internals/RecordsIterator.java:
##########
@@ -179,8 +182,13 @@ private Optional<Batch<T>> nextBatch() {
return Optional.empty();
}
- private Batch<T> readBatch(DefaultRecordBatch batch) {
+ private Batch<T> readBatch(DefaultRecordBatch batch) throws
CorruptRecordException {
final Batch<T> result;
+ if (doCrcValidation) {
Review Comment:
You can move this block before `final Batch<T> tresult` and keep `result`
close to where it is initialized.
##########
raft/src/main/java/org/apache/kafka/raft/internals/RecordsIterator.java:
##########
@@ -49,17 +50,20 @@
// Number of bytes from records read up to now
private int bytesRead = 0;
private boolean isClosed = false;
+ private boolean doCrcValidation = false;
Review Comment:
This should be `private final boolean doCrcValidation;`. It should be moved
with the rest of the `final` fields.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
