[GitHub] [kafka] jsancio commented on a diff in pull request #12457: WIP -- added some CRC checking

[GitBox <gi...@apache.org>]

jsancio commented on code in PR #12457:
URL: https://github.com/apache/kafka/pull/12457#discussion_r933320842


##########
core/src/main/scala/kafka/log/UnifiedLog.scala:
##########
@@ -1216,7 +1216,16 @@ class UnifiedLog(@volatile var logStartOffset: Long,
       case FetchHighWatermark => fetchHighWatermarkMetadata
       case FetchTxnCommitted => fetchLastStableOffsetMetadata
     }
-    localLog.read(startOffset, maxLength, minOneMessage, maxOffsetMetadata, isolation == FetchTxnCommitted)
+    val fetchDataInfo = localLog.read(startOffset, maxLength, minOneMessage, maxOffsetMetadata, isolation == FetchTxnCommitted)

Review Comment:
   Does this mean that Kafka needs to read all of the batches to handle a Fetch request? For performance, Kafka doesn't read the batches when handling a Fetch request. Kafka sends bytes. It is up to the reader (Consumer or Follower) to validate the bytes received.



##########
raft/src/main/java/org/apache/kafka/snapshot/RecordsSnapshotReader.java:
##########
@@ -106,6 +106,7 @@ public static <T> RecordsSnapshotReader<T> of(
         BufferSupplier bufferSupplier,
         int maxBatchSize
     ) {
+        // TODO: Is this a good place to perform delimeter check (i.e. existence of header and footer?)

Review Comment:
   If you do this here, that means you have to read the snapshot twice, right?



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org