You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/05/18 20:51:43 UTC

[GitHub] [kafka] mumrah opened a new pull request #10719: Fix compile errors for KAFKA-12543

mumrah opened a new pull request #10719:
URL: https://github.com/apache/kafka/pull/10719


   Fixes for two compiler errors which were introduced in https://github.com/apache/kafka/pull/10431


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

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

[GitHub] [kafka] mumrah merged pull request #10719: Fix compile errors for KAFKA-12543

Posted by GitBox <gi...@apache.org>.
mumrah merged pull request #10719:
URL: https://github.com/apache/kafka/pull/10719


   


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

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

[GitHub] [kafka] jsancio commented on pull request #10719: Fix compile errors for KAFKA-12543

Posted by GitBox <gi...@apache.org>.
jsancio commented on pull request #10719:
URL: https://github.com/apache/kafka/pull/10719#issuecomment-843561287


   LGTM. Compiles locally and `KafkaMetadataLogTest` passes locally. Thanks @mumrah 


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

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

[GitHub] [kafka] jsancio commented on a change in pull request #10719: Fix compile errors for KAFKA-12543

Posted by GitBox <gi...@apache.org>.
jsancio commented on a change in pull request #10719:
URL: https://github.com/apache/kafka/pull/10719#discussion_r634746833



##########
File path: raft/src/main/java/org/apache/kafka/snapshot/FileRawSnapshotReader.java
##########
@@ -63,7 +63,7 @@ public void close() {
                 String.format(
                     "Unable to close snapshot reader %s at %s",
                     snapshotId,
-                    fileRecords.file
+                    fileRecords

Review comment:
       I checked. `toString` is defined for `FileRecords`. It looks like it has enough information to debug the issue.




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

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

[GitHub] [kafka] junrao commented on a change in pull request #10719: Fix compile errors for KAFKA-12543

Posted by GitBox <gi...@apache.org>.
junrao commented on a change in pull request #10719:
URL: https://github.com/apache/kafka/pull/10719#discussion_r634743071



##########
File path: raft/src/main/java/org/apache/kafka/snapshot/FileRawSnapshotReader.java
##########
@@ -63,7 +63,7 @@ public void close() {
                 String.format(
                     "Unable to close snapshot reader %s at %s",
                     snapshotId,
-                    fileRecords.file
+                    fileRecords

Review comment:
       Do we need to define the toString() method for FileRecords so that it prints well?

##########
File path: core/src/main/scala/kafka/raft/KafkaMetadataLog.scala
##########
@@ -443,11 +443,12 @@ object KafkaMetadataLog {
 
   private def deleteSnapshotFiles(
     logDir: Path,
-    expiredSnapshots: mutable.TreeMap[OffsetAndEpoch, Option[FileRawSnapshotReader]]
+    expiredSnapshots: mutable.TreeMap[OffsetAndEpoch, Option[FileRawSnapshotReader]],
+    logging: Logging
   ): () => Unit = () => {
     expiredSnapshots.foreach { case (snapshotId, snapshotReader) =>
       snapshotReader.foreach { reader =>
-        CoreUtils.swallow(reader.close(), this)
+        CoreUtils.swallow(reader.close(), logging)

Review comment:
       This is an existing issue. But CoreUtils.swallow() doesn't seem to need logging.




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

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