Kafka
  1. Kafka
  2. KAFKA-830

partition replica assignment map in the controller should be a Set

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: controller
    • Labels:

      Description

      partitionReplicaAssignment currently stores the list of assigned replicas as a sequence. When a broker comes online, the replica state machine adds the broker to the list of assigned replicas. It should do that only if the replica is already not in the list of assigned replicas. This causes the replication factor to be incorrectly calculated

      1. kafka-830-v2.patch
        1 kB
        Neha Narkhede
      2. kafka-830-v1.patch
        26 kB
        Neha Narkhede

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        In Progress In Progress Open Open
        3s 1 Neha Narkhede 27/Mar/13 15:29
        Open Open In Progress In Progress
        15h 21m 2 Neha Narkhede 27/Mar/13 15:29
        In Progress In Progress Patch Available Patch Available
        4s 1 Neha Narkhede 27/Mar/13 15:29
        Patch Available Patch Available Resolved Resolved
        5h 4m 1 Neha Narkhede 27/Mar/13 20:34
        Resolved Resolved Closed Closed
        11s 1 Neha Narkhede 27/Mar/13 20:34
        Tony Stevenson made changes -
        Workflow Apache Kafka Workflow [ 13052705 ] no-reopen-closed, patch-avail [ 13055348 ]
        Tony Stevenson made changes -
        Workflow no-reopen-closed, patch-avail [ 12773648 ] Apache Kafka Workflow [ 13052705 ]
        Hide
        Neha Narkhede added a comment -

        Swapnil, thanks for pointing that out, we can remember to do this in 0.8.1

        Show
        Neha Narkhede added a comment - Swapnil, thanks for pointing that out, we can remember to do this in 0.8.1
        Neha Narkhede made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Neha Narkhede added a comment -

        Thanks for the review

        Show
        Neha Narkhede added a comment - Thanks for the review
        Neha Narkhede made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Jun Rao added a comment -

        +1 on patch v2.

        Show
        Jun Rao added a comment - +1 on patch v2.
        Hide
        Swapnil Ghike added a comment -

        collection.mutable.LinkedHashSet will iterate the elements in the same sequence they were inserted. But using it will require changes at more places than the one line change in patch v2.

        Show
        Swapnil Ghike added a comment - collection.mutable.LinkedHashSet will iterate the elements in the same sequence they were inserted. But using it will require changes at more places than the one line change in patch v2.
        Hide
        Neha Narkhede added a comment -

        Actually, thinking about it more, even picking the smallest replica id does not make sense. This is because if we pick the first replica to be the largest broker id, the rest of the replica ids are smaller than the preferred replica in this case. We will have to keep this a sequence and be careful while adding a replica id to the assigned replica list

        Show
        Neha Narkhede added a comment - Actually, thinking about it more, even picking the smallest replica id does not make sense. This is because if we pick the first replica to be the largest broker id, the rest of the replica ids are smaller than the preferred replica in this case. We will have to keep this a sequence and be careful while adding a replica id to the assigned replica list
        Neha Narkhede made changes -
        Attachment kafka-830-v2.patch [ 12575733 ]
        Hide
        Neha Narkhede added a comment -

        Good catch! However, it doesn't make sense to keep it sequence since by definition those ids are unique. As for preferred replica logic, currently it is just the smallest replica id . So there are 2 options -

        1. Store replica ids in a sorted set
        2. For preferred replica, pick the smallest id from a traditional hash set

        But in patch v2, I stuck to your suggestion since it is a much smaller change for 0.8. Will fix this in 0.8.1

        Show
        Neha Narkhede added a comment - Good catch! However, it doesn't make sense to keep it sequence since by definition those ids are unique. As for preferred replica logic, currently it is just the smallest replica id . So there are 2 options - 1. Store replica ids in a sorted set 2. For preferred replica, pick the smallest id from a traditional hash set But in patch v2, I stuck to your suggestion since it is a much smaller change for 0.8. Will fix this in 0.8.1
        Hide
        Jun Rao added a comment -

        Thanks for the patch. One problem with changing assigned replicas from a list to a set is that it breaks the preferred replica logic. We assume that the first replica is the preferred one. So the ordering of the replicas is important. Perhaps we should keep replicas as a list and make sure that we don't add duplicates when changing it.

        Show
        Jun Rao added a comment - Thanks for the patch. One problem with changing assigned replicas from a list to a set is that it breaks the preferred replica logic. We assume that the first replica is the preferred one. So the ordering of the replicas is important. Perhaps we should keep replicas as a list and make sure that we don't add duplicates when changing it.
        Neha Narkhede made changes -
        Status In Progress [ 3 ] Patch Available [ 10002 ]
        Neha Narkhede made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Neha Narkhede made changes -
        Status In Progress [ 3 ] Open [ 1 ]
        Neha Narkhede made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Neha Narkhede made changes -
        Attachment kafka-830-v1.patch [ 12575719 ]
        Hide
        Neha Narkhede added a comment -
        • Changed any replica list to a set. This makes sense since replica ids can never legitimately repeat.
        Show
        Neha Narkhede added a comment - Changed any replica list to a set. This makes sense since replica ids can never legitimately repeat.
        Neha Narkhede made changes -
        Field Original Value New Value
        Component/s controller [ 12320321 ]
        Neha Narkhede created issue -

          People

          • Assignee:
            Neha Narkhede
            Reporter:
            Neha Narkhede
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development