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

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

niket-goel commented on code in PR #12457:
URL: https://github.com/apache/kafka/pull/12457#discussion_r933489518


##########
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:
   That is the downside, yes. We could also seek to the end of the snapshot file and just read the first and last batch. More ideal w.r.t performance would be to just check for the existence of the footer as you are iterating through the snapshot. You would still realize that you have an incomplete snapshot, but you would have applied a part of that incomplete snapshot to your memory (not sure if that poses a correctness risk).



##########
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:
   That is a fair question. I was contemplating that too. The alternative of having a reader verify the bytes places less stress on the log layer in Kafka, but is not really fool proof.
   e.g. in that scheme the listeners that are reading the batches (both for controller and broker) would do the CRC check (assuming the batch header reaches that layer).
   Actually now that I think about it, does having the check in the `RecordsIterator` protect all readers?



-- 
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