jsancio commented on code in PR #12457:
URL: https://github.com/apache/kafka/pull/12457#discussion_r938007440
##########
raft/src/test/java/org/apache/kafka/raft/internals/RecordsIteratorTest.java:
##########
@@ -110,6 +114,15 @@ public void testCrcValidation(
// Verify check does not trigger when doCrcValidation is false
assertDoesNotThrow(() -> testIterator(batches, memRecords, false));
assertDoesNotThrow(() -> testIterator(batches, fileRecords, false));
+
+ // Fix the corruption
+ memRecords.buffer().putInt(DefaultRecordBatch.CRC_OFFSET, actualCrc);
+
+ // Verify check does not trigger when the corruption is fixed
+ assertDoesNotThrow(() -> testIterator(batches, memRecords, true));
+ FileRecords moreFileRecords = FileRecords.open(TestUtils.tempFile());
Review Comment:
This also applies to existing tests. The tests should close the
`FileRecords` objects when they are done using them.
##########
raft/src/test/java/org/apache/kafka/raft/internals/RecordsIteratorTest.java:
##########
@@ -98,8 +98,12 @@ public void testCrcValidation(
) 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());
+ // Read the Batch CRC for the first batch from the buffer
+ ByteBuffer readBuf = memRecords.buffer().asReadOnlyBuffer();
+ readBuf.position(DefaultRecordBatch.CRC_OFFSET);
+ Integer actualCrc = readBuf.getInt();
Review Comment:
How about this to avoid the boxing and object creation:
```java
int actualCrc = readBuf.getInt();
```
##########
raft/src/test/java/org/apache/kafka/raft/internals/RecordsIteratorTest.java:
##########
@@ -98,8 +98,12 @@ public void testCrcValidation(
) 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());
+ // Read the Batch CRC for the first batch from the buffer
+ ByteBuffer readBuf = memRecords.buffer().asReadOnlyBuffer();
Review Comment:
Btw, `adReadOnlyBuffer` is probably not needed since `MemoryBuffer#buffer`
returns a duplicate view into the buffer.
--
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]
