jsancio commented on code in PR #22963:
URL: https://github.com/apache/kafka/pull/22963#discussion_r4209480905
##########
clients/src/test/java/org/apache/kafka/common/record/internal/FileRecordsTest.java:
##########
@@ -84,6 +84,69 @@ public void cleanup() throws IOException {
this.fileRecords.close();
}
+ /**
+ * A buffer with more room than the records left from the given position
is filled only up to
+ * their end, so that the bytes which follow them in the file are not
read. Those bytes exist
+ * whenever the records are a slice of a larger file, or the segment was
preallocated.
+ */
+ @Test
+ public void testReadIntoStopsAtTheEndOfTheRecords() throws Exception {
+ // The bytes which follow a slice are usually more records rather than
zeros, in which case
+ // reading past it stays silent instead of failing, so cover both.
+ int firstBatchSize =
fileRecords.batches().iterator().next().sizeInBytes();
+ ByteBuffer sliced = ByteBuffer.allocate(1024);
+ fileRecords.slice(0, firstBatchSize).readInto(sliced, 0);
+ assertEquals(firstBatchSize, sliced.remaining());
+
+ // A slice which does not start at the beginning of the file reads
from its own start.
+ FileRecords tail = fileRecords.slice(firstBatchSize,
fileRecords.sizeInBytes() - firstBatchSize);
+ sliced.clear();
+ tail.readInto(sliced, 0);
+ assertEquals(fileRecords.sizeInBytes() - firstBatchSize,
sliced.remaining());
+ assertEquals(values.length - 1,
recordCount(MemoryRecords.readableRecords(sliced)));
+
+ try (FileRecords preallocated = FileRecords.open(tempFile(), false,
1024, true)) {
+ append(preallocated, values);
+ int sizeInBytes = preallocated.sizeInBytes();
+ // The file is longer than the records, otherwise this proves
nothing.
+ assertTrue(preallocated.file().length() > sizeInBytes);
Review Comment:
assertTrue allows you to print message when it fails. You can use this to
print the value of `length()` and sizeInBytes. E.g. "the file length (%s) is
not greater than size in bytes (%s)".
##########
clients/src/test/java/org/apache/kafka/common/record/internal/FileRecordsTest.java:
##########
@@ -84,6 +84,69 @@ public void cleanup() throws IOException {
this.fileRecords.close();
}
+ /**
+ * A buffer with more room than the records left from the given position
is filled only up to
+ * their end, so that the bytes which follow them in the file are not
read. Those bytes exist
+ * whenever the records are a slice of a larger file, or the segment was
preallocated.
+ */
+ @Test
+ public void testReadIntoStopsAtTheEndOfTheRecords() throws Exception {
+ // The bytes which follow a slice are usually more records rather than
zeros, in which case
+ // reading past it stays silent instead of failing, so cover both.
+ int firstBatchSize =
fileRecords.batches().iterator().next().sizeInBytes();
+ ByteBuffer sliced = ByteBuffer.allocate(1024);
Review Comment:
You can allocate the buffer using the first batch size:
```java
ByteBuffer buffer = ByteBuffer.allocate(firstBatchSize * 2);
```
##########
clients/src/test/java/org/apache/kafka/common/record/internal/FileRecordsTest.java:
##########
@@ -84,6 +84,69 @@ public void cleanup() throws IOException {
this.fileRecords.close();
}
+ /**
+ * A buffer with more room than the records left from the given position
is filled only up to
+ * their end, so that the bytes which follow them in the file are not
read. Those bytes exist
+ * whenever the records are a slice of a larger file, or the segment was
preallocated.
+ */
+ @Test
+ public void testReadIntoStopsAtTheEndOfTheRecords() throws Exception {
+ // The bytes which follow a slice are usually more records rather than
zeros, in which case
+ // reading past it stays silent instead of failing, so cover both.
+ int firstBatchSize =
fileRecords.batches().iterator().next().sizeInBytes();
+ ByteBuffer sliced = ByteBuffer.allocate(1024);
+ fileRecords.slice(0, firstBatchSize).readInto(sliced, 0);
+ assertEquals(firstBatchSize, sliced.remaining());
+
+ // A slice which does not start at the beginning of the file reads
from its own start.
+ FileRecords tail = fileRecords.slice(firstBatchSize,
fileRecords.sizeInBytes() - firstBatchSize);
+ sliced.clear();
Review Comment:
Let's just create a new buffer with the size that you need for this test.
--
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]
