Kafka
  1. Kafka
  2. KAFKA-575

Partition.makeFollower() reads broker info from ZK

    Details

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

      Description

      To follow a new leader, Partition.makeFollower() has to obtain the broker info of the new leader. Currently, it reads that info from ZK for every affected partition. This increases the time for a leader to truly available.

      1. kafka-575-v3-correct.patch
        17 kB
        Swapnil Ghike
      2. kafka-575-v3.patch
        0.3 kB
        Swapnil Ghike
      3. kafka-575-v2.patch
        17 kB
        Swapnil Ghike
      4. kafka-575-v1.patch
        17 kB
        Swapnil Ghike

        Activity

        Hide
        Jun Rao added a comment -

        Thanks for patch v3. +1. Committed to 0.8.

        Show
        Jun Rao added a comment - Thanks for patch v3. +1. Committed to 0.8.
        Hide
        Swapnil Ghike added a comment -

        Sorry, please ignore the last patch. This one is the correct one.

        Show
        Swapnil Ghike added a comment - Sorry, please ignore the last patch. This one is the correct one.
        Hide
        Swapnil Ghike added a comment -

        Minor change + passed sanity test.

        Show
        Swapnil Ghike added a comment - Minor change + passed sanity test.
        Hide
        Swapnil Ghike added a comment -

        Made the change.

        Show
        Swapnil Ghike added a comment - Made the change.
        Hide
        Jun Rao added a comment -

        Patch looks good. Just one comment:

        1. ControllerBrokerRequestBatch.sendRequestsToBrokers(): Instead of including all live brokers in the leaderAndIsr request, we probably can just include brokers that are leaders in each LeaderAndIsr request.

        Show
        Jun Rao added a comment - Patch looks good. Just one comment: 1. ControllerBrokerRequestBatch.sendRequestsToBrokers(): Instead of including all live brokers in the leaderAndIsr request, we probably can just include brokers that are leaders in each LeaderAndIsr request.
        Hide
        Swapnil Ghike added a comment -

        1. Live broker info included in LeaderIsrRequest.

        ControllerBrokerRequestBatch.sendrequestsToBrokers() passes the live brokers list to LeaderIsrRequest.

        Thus Partition.makeFollower no longer needs to read the broker info from zookeeper. Only the controller reads this info from zookeeper with this patch.

        2. Partition.makeLeaderOrFollower feels unnecessary because it reuses only a couple lines of code, and introduces an extra step in understanding the logic.

        Hence, got rid of makeLeaderOrFollower and copied the lock synchronization and discarding the incoming request part to makeLeader and makeFollower separately.

        Show
        Swapnil Ghike added a comment - 1. Live broker info included in LeaderIsrRequest. ControllerBrokerRequestBatch.sendrequestsToBrokers() passes the live brokers list to LeaderIsrRequest. Thus Partition.makeFollower no longer needs to read the broker info from zookeeper. Only the controller reads this info from zookeeper with this patch. 2. Partition.makeLeaderOrFollower feels unnecessary because it reuses only a couple lines of code, and introduces an extra step in understanding the logic. Hence, got rid of makeLeaderOrFollower and copied the lock synchronization and discarding the incoming request part to makeLeader and makeFollower separately.
        Hide
        Jun Rao added a comment -

        One way to address this issue is to include the broker info in the leaderAndIsr request. This way, Partition.makeFollower() doesn't need to read from ZK any more.

        Show
        Jun Rao added a comment - One way to address this issue is to include the broker info in the leaderAndIsr request. This way, Partition.makeFollower() doesn't need to read from ZK any more.

          People

          • Assignee:
            Swapnil Ghike
            Reporter:
            Jun Rao
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 48h
              48h
              Remaining:
              Remaining Estimate - 48h
              48h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development