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/06/15 00:27:16 UTC

[GitHub] [kafka] IgnacioAcunaF opened a new pull request #10883: KAFKA-12949: Add catch case to avoid scala.MatchError null

IgnacioAcunaF opened a new pull request #10883:
URL: https://github.com/apache/kafka/pull/10883


   Encounter the following exception when trying to run the TestRaftServer:
   
   `bin/test-kraft-server-start.sh --config config/kraft.properties`
   ```
   [2021-06-14 17:15:43,232] ERROR [raft-workload-generator]: Error due to (kafka.tools.TestRaftServer$RaftWorkloadGenerator)
    scala.MatchError: null
    at kafka.tools.TestRaftServer$RaftWorkloadGenerator.doWork(TestRaftServer.scala:220)
    at kafka.utils.ShutdownableThread.run(ShutdownableThread.scala:96)
    [2021-06-14 17:15:43,253] INFO [raft-workload-generator]: Stopped (kafka.tools.TestRaftServer$RaftWorkloadGenerator)
   ```
   
   Caused because of a not contemplated *null* on eventQueue.poll:
   
   ```
   eventQueue.poll(eventTimeoutMs, TimeUnit.MILLISECONDS) match {
       case HandleClaim(epoch) =>
         claimedEpoch = Some(epoch)
         throttler.reset()
         pendingAppends.clear()
         recordCount.set(0)
   
       case HandleResign =>
         claimedEpoch = None
         pendingAppends.clear()
   
       case HandleCommit(reader) =>
         try {
           while (reader.hasNext) {
             val batch = reader.next()
             claimedEpoch.foreach { leaderEpoch =>
               handleLeaderCommit(leaderEpoch, batch)
             }
           }
         } finally {
           reader.close()
         }
   
       case HandleSnapshot(reader) =>
         // Ignore snapshots; only interested in records appended by this leader
         reader.close()
   
       case Shutdown => // Ignore shutdown command
     }
   ```
   This makes raft-workload-generator's thread to stop.
   
   Proposal: 
   Add a catch case on the match statement.
    
   `case _ => // Ignore other events (such as null)`
   
   ```
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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] IgnacioAcunaF commented on pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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


   With no change:
   ![image](https://user-images.githubusercontent.com/31544929/121976023-f49deb80-cd50-11eb-8fa7-94b7c4923751.png)
   
   With the change:
   ![image](https://user-images.githubusercontent.com/31544929/121975932-c5877a00-cd50-11eb-8a4a-bf930a1982dc.png)
   


-- 
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] IgnacioAcunaF commented on a change in pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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



##########
File path: core/src/main/scala/kafka/tools/TestRaftServer.scala
##########
@@ -238,6 +238,8 @@ class TestRaftServer(
           reader.close()
 
         case Shutdown => // Ignore shutdown command
+
+        case _ => // Ignore other events (such as null)

Review comment:
       Exactly. When when eventQueue hits the eventTimeoutMs the `null` value arises (as there is nothing to poll from). When debugging encounter that `eventTimeoutMs ` could be 0 (if the throttleTimeMs is equal to 0), so if there is no inmediate events on the eventQueue, the `null` arises inmediatly, and as it is not caught, the thread stops.
   
   And yes, at the beggining I approached it with a `case null => // Ignore null event`, which made the class run just fine, but let it generic to catch some events that may be being overlooked. 
   What do you think about the change?




-- 
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] IgnacioAcunaF commented on a change in pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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



##########
File path: core/src/main/scala/kafka/tools/TestRaftServer.scala
##########
@@ -238,6 +238,8 @@ class TestRaftServer(
           reader.close()
 
         case Shutdown => // Ignore shutdown command
+
+        case _ => // Ignore other events (such as null)

Review comment:
       Exactly. When when eventQueue hits the eventTimeoutMs the `null` value arises (as there is nothing to poll from). When debugging encounter that `eventTimeoutMs ` could be 0 (if the throttleTimeMs is equal to 0), so if there is no inmediate events on the eventQueue, the `null` arises inmediatly.
   
   And yes, at the beggining I approached it with a `case null => // Ignore null event`, which made the class run just fine, but let it generic to catch some events that may be being overlooked.




-- 
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] IgnacioAcunaF edited a comment on pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

Posted by GitBox <gi...@apache.org>.
IgnacioAcunaF edited a comment on pull request #10883:
URL: https://github.com/apache/kafka/pull/10883#issuecomment-861696913


   @ijuma @jsancio  Thanks for your comments and review. Updated the PR


-- 
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] IgnacioAcunaF commented on pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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


   PING @ijuma (as I saw you on a PR related to test-kraft-server-start.sh [KAFKA-12672])


-- 
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 #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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



##########
File path: core/src/main/scala/kafka/tools/TestRaftServer.scala
##########
@@ -238,6 +238,8 @@ class TestRaftServer(
           reader.close()
 
         case Shutdown => // Ignore shutdown command
+
+        case _ => // Ignore other events (such as null)

Review comment:
       I assume that this value is `null` when the timeout is hit. If that is the case then make the case `case null => // Ignore null when timeout expires`.
   
   This should both handle the case when it is null and give us an error if all of the cases/type are not handled.




-- 
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] IgnacioAcunaF commented on pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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






-- 
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] IgnacioAcunaF commented on a change in pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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



##########
File path: core/src/main/scala/kafka/tools/TestRaftServer.scala
##########
@@ -238,6 +238,8 @@ class TestRaftServer(
           reader.close()
 
         case Shutdown => // Ignore shutdown command
+
+        case _ => // Ignore other events (such as null)

Review comment:
       Will change on the PR, thanks for the comment!




-- 
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] hachikuji merged pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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


   


-- 
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] IgnacioAcunaF edited a comment on pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

Posted by GitBox <gi...@apache.org>.
IgnacioAcunaF edited a comment on pull request #10883:
URL: https://github.com/apache/kafka/pull/10883#issuecomment-861696913


   @ijuma @jsancio  Thanks for your comments and review. Update the PR


-- 
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] IgnacioAcunaF commented on pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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


   Thanks for your comments and review. Update the PR


-- 
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] ijuma commented on pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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


   @hachikuji @jsancio do we want to keep this class? It's the second time it breaks so we either need to improve test coverage or we should remove it.


-- 
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] IgnacioAcunaF commented on a change in pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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



##########
File path: core/src/main/scala/kafka/tools/TestRaftServer.scala
##########
@@ -238,6 +238,8 @@ class TestRaftServer(
           reader.close()
 
         case Shutdown => // Ignore shutdown command
+
+        case _ => // Ignore other events (such as null)

Review comment:
       Exactly. When when eventQueue hits the eventTimeoutMs the `null` value arises (as there is nothing to poll from). When debugging encounter that `eventTimeoutMs ` could be 0 (if the throttleTimeMs is equal to 0), so if there is no inmediate events on the eventQueue, the `null` arises inmediatly, and as it is not caught, the thread stops.
   
   And yes, at the beggining I approached it with a `case null => // Ignore null event`, which made the class run just fine, but decided to let it generic to catch some events that may be being overlooked. 
   What do you think about that?




-- 
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] IgnacioAcunaF commented on a change in pull request #10883: KAFKA-12949: TestRaftServer's scala.MatchError: null on test-kraft-server-start.sh

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



##########
File path: core/src/main/scala/kafka/tools/TestRaftServer.scala
##########
@@ -238,6 +238,8 @@ class TestRaftServer(
           reader.close()
 
         case Shutdown => // Ignore shutdown command
+
+        case _ => // Ignore other events (such as null)

Review comment:
       Exactly. When when eventQueue hits the eventTimeoutMs the `null` value arises (as there is nothing to poll from). When debugging encounter that `eventTimeoutMs ` could be 0 (if the throttleTimeMs is equal to 0), so if there is no inmediate events on the eventQueue, the `null` arises inmediatly, and as it is not caught, the thread stops.
   
   And yes, at the beggining I approached it with a `case null => // Ignore null event`, which made the class run just fine, but decided to let it generic to catch some events that may be being overlooked. Thinking of it know seems better to just let the case null, because otherwise it would made more difficult to debug further situations (non mapped events for example)




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