Kafka
  1. Kafka
  2. KAFKA-1098

Unit test failure in 0.8.1 related to LogCleaner

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.1
    • Fix Version/s: None
    • Component/s: log
    • Labels:
      None

      Description

      Floor = 0, To = -1
      [2013-10-22 09:39:25,001] ERROR Error in cleaner thread 0: (kafka.log.LogCleaner:103)
      java.lang.IllegalArgumentException: inconsistent range
      at java.util.concurrent.ConcurrentSkipListMap$SubMap.<init>(ConcurrentSkipListMap.java:2506)
      at java.util.concurrent.ConcurrentSkipListMap.subMap(ConcurrentSkipListMap.java:1984)
      at kafka.log.Log.logSegments(Log.scala:605)
      at kafka.log.LogToClean.<init>(LogCleaner.scala:596)
      at kafka.log.LogCleaner$$anonfun$5.apply(LogCleaner.scala:137)
      at kafka.log.LogCleaner$$anonfun$5.apply(LogCleaner.scala:137)
      at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:206)
      at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:206)
      at scala.collection.LinearSeqOptimized$class.foreach(LinearSeqOptimized.scala:61)
      at scala.collection.immutable.List.foreach(List.scala:45)
      at scala.collection.TraversableLike$class.map(TraversableLike.scala:206)
      at scala.collection.immutable.List.map(List.scala:45)
      at kafka.log.LogCleaner.kafka$log$LogCleaner$$grabFilthiestLog(LogCleaner.scala:137)
      at kafka.log.LogCleaner$CleanerThread.cleanOrSleep(LogCleaner.scala:203)
      at kafka.log.LogCleaner$CleanerThread.run(LogCleaner.scala:189)

      1. KAFKA-1098-v3.patch
        2 kB
        Jay Kreps
      2. KAFKA-1098-v2.patch
        2 kB
        Jay Kreps
      3. kafka_1098-v1.patch
        1 kB
        Joris Van Remoortere

        Activity

        Jay Kreps made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Jun Rao added a comment -

        +1 for v3.

        To compile in all versions of scala, do the following. It's included in README.md
        ./sbt +package

        Show
        Jun Rao added a comment - +1 for v3. To compile in all versions of scala, do the following. It's included in README.md ./sbt +package
        Jay Kreps made changes -
        Attachment KAFKA-1098-v3.patch [ 12610260 ]
        Hide
        Jay Kreps added a comment -

        Ack, fixed 2.10 issue. Frustrating. Do we have a single sbt command that can compile for all?

        Jun--the other two null comparisons are for entries not offsets so == should be valid there.

        Show
        Jay Kreps added a comment - Ack, fixed 2.10 issue. Frustrating. Do we have a single sbt command that can compile for all? Jun--the other two null comparisons are for entries not offsets so == should be valid there.
        Hide
        Neha Narkhede added a comment -

        Joris Van Remoortere That list of Scala versions is correct.

        Show
        Neha Narkhede added a comment - Joris Van Remoortere That list of Scala versions is correct.
        Hide
        Joris Van Remoortere added a comment -

        Is the list of Scala versions in the README file (2.8.0, 2.8.2, 2.9.1, 2.9.2 or 2.10.1) a full and accurate representation of the versions we intend to support? I can be more thorough in running the test suite under each one, but am not sure which versions I need to test.

        Show
        Joris Van Remoortere added a comment - Is the list of Scala versions in the README file (2.8.0, 2.8.2, 2.9.1, 2.9.2 or 2.10.1) a full and accurate representation of the versions we intend to support? I can be more thorough in running the test suite under each one, but am not sure which versions I need to test.
        Hide
        Jun Rao added a comment -

        Thanks for the patch. It doesn't compile with scala 2.10.1 though since asIterable no longer exists there. I guess we have to import all JavaConversions._ to get around the cross compilation issue.
        [error] /Users/jrao/Intellij/kafka_git/core/src/main/scala/kafka/log/Log.scala:589: value asIterable is not a member of object scala.collection.JavaConversions
        [error] import JavaConversions.asIterable
        [error] ^

        Also, in the following places, should we use eq to test null for the returned entry from the skipListMap?

        In read(),
        if(startOffset > next || entry == null)

        In roll(),
        segments.lastEntry() match

        { case null => case entry => entry.getValue.index.trimToValidSize() }
        Show
        Jun Rao added a comment - Thanks for the patch. It doesn't compile with scala 2.10.1 though since asIterable no longer exists there. I guess we have to import all JavaConversions._ to get around the cross compilation issue. [error] /Users/jrao/Intellij/kafka_git/core/src/main/scala/kafka/log/Log.scala:589: value asIterable is not a member of object scala.collection.JavaConversions [error] import JavaConversions.asIterable [error] ^ Also, in the following places, should we use eq to test null for the returned entry from the skipListMap? In read(), if(startOffset > next || entry == null) In roll(), segments.lastEntry() match { case null => case entry => entry.getValue.index.trimToValidSize() }
        Hide
        Jay Kreps added a comment -

        Sorry about the churn btw I was only compiling for 2.9 which obviously isn't enough.

        Show
        Jay Kreps added a comment - Sorry about the churn btw I was only compiling for 2.9 which obviously isn't enough.
        Jay Kreps made changes -
        Attachment KAFKA-1098-v2.patch [ 12609802 ]
        Hide
        Jay Kreps added a comment -

        Here is a patch that seems to fix the issue.

        Show
        Jay Kreps added a comment - Here is a patch that seems to fix the issue.
        Hide
        Jay Kreps added a comment -

        Joel, I think you're right. I can grab this.

        Show
        Jay Kreps added a comment - Joel, I think you're right. I can grab this.
        Hide
        Neha Narkhede added a comment -

        +1 on Joel Koshy's suggestion. Joris Van Remoortere Would you like to take a stab at that?

        Show
        Neha Narkhede added a comment - +1 on Joel Koshy 's suggestion. Joris Van Remoortere Would you like to take a stab at that?
        Hide
        Joris Van Remoortere added a comment -

        I think the behavior is dependent on the version of Scala used. When I first wrote the patch for Kafka-1042 I was using 2.9.2. This error seems to arise when using 2.8.0.
        Your suggestion makes sense, I'm not sure what the plan is for supporting older versions of Scala.

        Show
        Joris Van Remoortere added a comment - I think the behavior is dependent on the version of Scala used. When I first wrote the patch for Kafka-1042 I was using 2.9.2. This error seems to arise when using 2.8.0. Your suggestion makes sense, I'm not sure what the plan is for supporting older versions of Scala.
        Hide
        Joel Koshy added a comment -

        Thanks for the patch - this is interesting/weird.

        The real issue seems to be that since the map is defined as [Long, LogSegment] the null return of floorKey is getting converted to a 0 Long value and failing the eq null check.

        i.e., the actual from value is also -1. floorKey should return null (and it does) but it is implicitly converted to 0, so it does not enter the floor eq null block but we want it to.

        I'm wondering if we should just switch the map to use java.lang.Long instead of scala Long to avoid these implicit conversions.

        scala> val m = new ConcurrentSkipListMap[Long, Any]
        m: java.util.concurrent.ConcurrentSkipListMap[Long,Any] = {}
        
        scala> m.floorKey(0)
        res10: Long = 0
        
        scala> val m = new ConcurrentSkipListMap[java.lang.Long, Any]
        m: java.util.concurrent.ConcurrentSkipListMap[java.lang.Long,Any] = {}
        
        scala> m.floorKey(0)
        res11: java.lang.Long = null
        
        Show
        Joel Koshy added a comment - Thanks for the patch - this is interesting/weird. The real issue seems to be that since the map is defined as [Long, LogSegment] the null return of floorKey is getting converted to a 0 Long value and failing the eq null check. i.e., the actual from value is also -1. floorKey should return null (and it does) but it is implicitly converted to 0, so it does not enter the floor eq null block but we want it to. I'm wondering if we should just switch the map to use java.lang.Long instead of scala Long to avoid these implicit conversions. scala> val m = new ConcurrentSkipListMap[ Long , Any] m: java.util.concurrent.ConcurrentSkipListMap[ Long ,Any] = {} scala> m.floorKey(0) res10: Long = 0 scala> val m = new ConcurrentSkipListMap[java.lang. Long , Any] m: java.util.concurrent.ConcurrentSkipListMap[java.lang. Long ,Any] = {} scala> m.floorKey(0) res11: java.lang. Long = null
        Joris Van Remoortere made changes -
        Field Original Value New Value
        Attachment kafka_1098-v1.patch [ 12609702 ]
        Hide
        Joris Van Remoortere added a comment -

        logSegments can currently be called with to = -1. This can trigger submap to be called with invalid arguments (i.e. from > to). We catch this case and return an empty iterable of logSegments.

        Show
        Joris Van Remoortere added a comment - logSegments can currently be called with to = -1. This can trigger submap to be called with invalid arguments (i.e. from > to). We catch this case and return an empty iterable of logSegments.
        Neha Narkhede created issue -

          People

          • Assignee:
            Jay Kreps
            Reporter:
            Neha Narkhede
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development