Kafka
  1. Kafka
  2. KAFKA-1469

Util.abs function does not return correct absolute values for negative values

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.2.0
    • Component/s: None
    • Labels:

      Description

      Reported by Russell Melick. [edit1: I don't think this affects correctness of
      the places that use the abs utility since we just need it to return a
      consistent positive value, but we should fix this nonetheless]
      [edit 2: actually it affects correctness in places that depend on consistent
      values across the fix. e.g., the offset manager is determined based on
      abs(hash(consumer group)). So after an upgrade that can change]

           /**
            * Get the absolute value of the given number. If the number is
         Int.MinValue return 0.
            * This is different from java.lang.Math.abs or scala.math.abs in that
         they return Int.MinValue (!).
            */
           def abs(n: Int) = n & 0x7fffffff
      

      For negative integers, it does not return the absolute value. It does
      appear to do what the comment says for Int.MinValue though. For example,

         scala> -1 & 0x7fffffff
         res8: Int = 2147483647
      
         scala> -2 & 0x7fffffff
         res9: Int = 2147483646
      
         scala> -2147483647 & 0x7fffffff
         res11: Int = 1
      
         scala> -2147483648 & 0x7fffffff
         res12: Int = 0
      
      1. KAFKA-1469.patch
        1 kB
        Sebastian Geller

        Activity

        Joel Koshy created issue -
        Sebastian Geller made changes -
        Field Original Value New Value
        Attachment KAFKA-1469.patch [ 12648224 ]
        Hide
        Sebastian Geller added a comment -

        This patch handles MinValue as described in the comment aswell as handling negative integers. Added unit tests nevertheless.

        Show
        Sebastian Geller added a comment - This patch handles MinValue as described in the comment aswell as handling negative integers. Added unit tests nevertheless.
        Sebastian Geller made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Labels newbie newbie patch
        Hide
        Guozhang Wang added a comment -

        Thanks for the patch, I am +1 on this one.

        Show
        Guozhang Wang added a comment - Thanks for the patch, I am +1 on this one.
        Hide
        Neha Narkhede added a comment -

        +1. Thanks for the patch.

        Show
        Neha Narkhede added a comment - +1. Thanks for the patch.
        Hide
        Neha Narkhede added a comment -

        Pushed to trunk

        Show
        Neha Narkhede added a comment - Pushed to trunk
        Neha Narkhede made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Neha Narkhede made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Joe Stein made changes -
        Fix Version/s 0.8.2 [ 12326167 ]
        Joel Koshy made changes -
        Description Reported by Russell Melick. [edit: I don't think this affects correctness of
        the places that use the abs utility since we just need it to return a
        consistent positive value, but we should fix this nonetheless]

        {code}
             /**
              * Get the absolute value of the given number. If the number is
           Int.MinValue return 0.
              * This is different from java.lang.Math.abs or scala.math.abs in that
           they return Int.MinValue (!).
              */
             def abs(n: Int) = n & 0x7fffffff
        {code}

        For negative integers, it does not return the absolute value. It does
        appear to do what the comment says for Int.MinValue though. For example,

        {code}
           scala> -1 & 0x7fffffff
           res8: Int = 2147483647

           scala> -2 & 0x7fffffff
           res9: Int = 2147483646

           scala> -2147483647 & 0x7fffffff
           res11: Int = 1

           scala> -2147483648 & 0x7fffffff
           res12: Int = 0
        {code}
        Reported by Russell Melick. [edit1: I don't think this affects correctness of
        the places that use the abs utility since we just need it to return a
        consistent positive value, but we should fix this nonetheless]
        [edit 2: actually it affects correctness in places that depend on consistent
        values across the fix. e.g., the offset manager is determined based on
        abs(hash(consumer group)). So after an upgrade that can change]

        {code}
             /**
              * Get the absolute value of the given number. If the number is
           Int.MinValue return 0.
              * This is different from java.lang.Math.abs or scala.math.abs in that
           they return Int.MinValue (!).
              */
             def abs(n: Int) = n & 0x7fffffff
        {code}

        For negative integers, it does not return the absolute value. It does
        appear to do what the comment says for Int.MinValue though. For example,

        {code}
           scala> -1 & 0x7fffffff
           res8: Int = 2147483647

           scala> -2 & 0x7fffffff
           res9: Int = 2147483646

           scala> -2147483647 & 0x7fffffff
           res11: Int = 1

           scala> -2147483648 & 0x7fffffff
           res12: Int = 0
        {code}
        Tony Stevenson made changes -
        Workflow no-reopen-closed, patch-avail [ 12864566 ] Apache Kafka Workflow [ 13051241 ]
        Tony Stevenson made changes -
        Workflow Apache Kafka Workflow [ 13051241 ] no-reopen-closed, patch-avail [ 13054576 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        10d 22h 39m 1 Sebastian Geller 03/Jun/14 20:33
        Patch Available Patch Available Resolved Resolved
        2d 20h 30m 1 Neha Narkhede 06/Jun/14 17:04
        Resolved Resolved Closed Closed
        6s 1 Neha Narkhede 06/Jun/14 17:04

          People

          • Assignee:
            Unassigned
            Reporter:
            Joel Koshy
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development