Details

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

      Description

      It would be nice to have Kafka brokers auto-assign node ids rather than having that be a configuration. Having a configuration is irritating because (1) you have to generate a custom config for each broker and (2) even though it is in configuration, changing the node id can cause all kinds of bad things to happen.

      1. KAFKA-1070.patch
        205 kB
        Sriharsha Chintalapani
      2. KAFKA-1070_2015-01-12_18:30:17.patch
        29 kB
        Sriharsha Chintalapani
      3. KAFKA-1070_2015-01-12_10:46:54.patch
        28 kB
        Sriharsha Chintalapani
      4. KAFKA-1070_2015-01-01_17:39:30.patch
        28 kB
        Sriharsha Chintalapani
      5. KAFKA-1070_2014-11-25_20:29:37.patch
        43 kB
        Sriharsha Chintalapani
      6. KAFKA-1070_2014-11-20_10:50:04.patch
        35 kB
        Sriharsha Chintalapani
      7. KAFKA-1070_2014-08-21_10:26:20.patch
        47 kB
        Sriharsha Chintalapani
      8. KAFKA-1070_2014-07-24_21:05:33.patch
        22 kB
        Sriharsha Chintalapani
      9. KAFKA-1070_2014-07-24_20:58:17.patch
        33 kB
        Sriharsha Chintalapani
      10. KAFKA-1070_2014-07-22_11:34:18.patch
        35 kB
        Sriharsha Chintalapani
      11. KAFKA-1070_2014-07-19_16:06:13.patch
        18 kB
        Sriharsha Chintalapani

        Issue Links

          Activity

          Hide
          Jay Kreps added a comment -

          I think the right way to do this is to have a sequence in zookeeper that atomically increments and use this for id generation. On startup a node that has no id can generate one for itself and store it.

          One tricky bit is that this node id needs to be stored with the data, but we actually partition data up over multiple disks now and hope to be able to survive the destruction of any of them. Which disk should we store the node id on? I would recommend we store it on all of them--if it is missing on some we will add it there, if the id is inconsistent between disks we will error out (this should never happen).

          I would recommend adding a properties file named "meta" in every data directory containing the "id=x" value, we can extend this later with more perminant values. For example, I think it would be nice to add a data format version to help with in-place data upgrades.

          On startup the broker would check this value for consistency across directories. If it is not present in any directory it would auto-generate a node id and persist that for future use.

          For compatibility we would retain the current id configuration value--if it is present we will use it and ensure the id sequence is larger than this value.

          Show
          Jay Kreps added a comment - I think the right way to do this is to have a sequence in zookeeper that atomically increments and use this for id generation. On startup a node that has no id can generate one for itself and store it. One tricky bit is that this node id needs to be stored with the data, but we actually partition data up over multiple disks now and hope to be able to survive the destruction of any of them. Which disk should we store the node id on? I would recommend we store it on all of them--if it is missing on some we will add it there, if the id is inconsistent between disks we will error out (this should never happen). I would recommend adding a properties file named "meta" in every data directory containing the "id=x" value, we can extend this later with more perminant values. For example, I think it would be nice to add a data format version to help with in-place data upgrades. On startup the broker would check this value for consistency across directories. If it is not present in any directory it would auto-generate a node id and persist that for future use. For compatibility we would retain the current id configuration value--if it is present we will use it and ensure the id sequence is larger than this value.
          Hide
          Ancoron Luciferis added a comment -

          For me, a solution that generates a unique ID by itself would also be fine.

          E.g. as a workaround, I currently generate an integer based on the MAC address from the interface that clients are configured to see. However, this is not reliably unique, because a MAC address is 48-bit and the broker-ID is only a signed 32-bit integer, a long would be better fit here.

          Going further on standalone generation of unique IDs, within applications I am used to rely on something that does provide me some guarantees. A UUID of Type 1 (MAC/timestamp-based) is exactly that. However, here the problem is even worse, because a UUID is a 128-bit number.

          My question here would be (and I am not yet very much into the Kafka code base - so please excuse if this is a very dumb question): what is the brokerId actually being used for? In other words: which requirements does it have to fulfill and what functionality relies on it being unique inside a cluster?

          Another question is: why does it have to be a non-negative integer?

          Show
          Ancoron Luciferis added a comment - For me, a solution that generates a unique ID by itself would also be fine. E.g. as a workaround, I currently generate an integer based on the MAC address from the interface that clients are configured to see. However, this is not reliably unique, because a MAC address is 48-bit and the broker-ID is only a signed 32-bit integer, a long would be better fit here. Going further on standalone generation of unique IDs, within applications I am used to rely on something that does provide me some guarantees. A UUID of Type 1 (MAC/timestamp-based) is exactly that. However, here the problem is even worse, because a UUID is a 128-bit number. My question here would be (and I am not yet very much into the Kafka code base - so please excuse if this is a very dumb question): what is the brokerId actually being used for? In other words: which requirements does it have to fulfill and what functionality relies on it being unique inside a cluster? Another question is: why does it have to be a non-negative integer?
          Hide
          Sriharsha Chintalapani added a comment -

          Jay Kreps Neha Narkhede
          I am working on this JIRA. Following the proposed design in the comments. I see there might be a issue where
          one of the broker configs have broker.id defined as 0 and another one doesn't have broker.id. If the second broker started first we fetch a global sequence id from zookeeper that starts with 0 and the next one has already have a defined config we will use that in this case we have two brokers with same id. Should we consider this case in which a kafka cluster's broker might have a inconsistent config interms of broker.id.
          Instead of using zookeeper for generating a global sequence number why shouldn't we be using UUID and make brokerId Long type.
          Thanks.

          Show
          Sriharsha Chintalapani added a comment - Jay Kreps Neha Narkhede I am working on this JIRA. Following the proposed design in the comments. I see there might be a issue where one of the broker configs have broker.id defined as 0 and another one doesn't have broker.id. If the second broker started first we fetch a global sequence id from zookeeper that starts with 0 and the next one has already have a defined config we will use that in this case we have two brokers with same id. Should we consider this case in which a kafka cluster's broker might have a inconsistent config interms of broker.id. Instead of using zookeeper for generating a global sequence number why shouldn't we be using UUID and make brokerId Long type. Thanks.
          Hide
          Jay Kreps added a comment -

          Hey sriharsha chintalapani, that is a great point.

          I'd like to avoid changing the type of the id to a long or UUID so as to not have to bump up the protocol format for the metadata request which hands these out to clients (we would need a way to handle compatibility with older clients that don't expect the longer types).

          I think we can get around the problem you point out by just defaulting the node id sequence to 1000. This could theoretically conflict but most people number from 0 or 1 and we can discuss this in the release notes. Our plan will be to release with support for both configured node ids and assigned node ids for compatibility. After a couple of releases we will remove the config.

          So the behavior would be this:
          If there is a node id in the config we will validate it against the node id in the data directory
          If it matches that good, we'll use that.
          If it doesn't match that is bad, we'll crash with an error.
          If there is a node id in the data directory but none in the config, we'll use whatever is in the data directory.
          If there is no node id in the data directory yet but there is one in the config we'll write that to the data directory and use it.
          If there is neither a node id in the data directory nor in the config we'll allocate a node id and write it to the data directory.

          Show
          Jay Kreps added a comment - Hey sriharsha chintalapani , that is a great point. I'd like to avoid changing the type of the id to a long or UUID so as to not have to bump up the protocol format for the metadata request which hands these out to clients (we would need a way to handle compatibility with older clients that don't expect the longer types). I think we can get around the problem you point out by just defaulting the node id sequence to 1000. This could theoretically conflict but most people number from 0 or 1 and we can discuss this in the release notes. Our plan will be to release with support for both configured node ids and assigned node ids for compatibility. After a couple of releases we will remove the config. So the behavior would be this: If there is a node id in the config we will validate it against the node id in the data directory If it matches that good, we'll use that. If it doesn't match that is bad, we'll crash with an error. If there is a node id in the data directory but none in the config, we'll use whatever is in the data directory. If there is no node id in the data directory yet but there is one in the config we'll write that to the data directory and use it. If there is neither a node id in the data directory nor in the config we'll allocate a node id and write it to the data directory.
          Hide
          Sriharsha Chintalapani added a comment -

          Created reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Created reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Thanks Jay Kreps

          Show
          Sriharsha Chintalapani added a comment - Thanks Jay Kreps
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Clark Haskins added a comment -

          We currently have node IDs in the 0-10000 range. I think some better logic for identifying used node IDs is necessary rather than just starting from 1000.

          Show
          Clark Haskins added a comment - We currently have node IDs in the 0-10000 range. I think some better logic for identifying used node IDs is necessary rather than just starting from 1000.
          Hide
          Jon Bringhurst added a comment - - edited

          Clark Haskins, Sriharsha Chintalapani, having ReservedBrokerIdMaxValue as a configurable option (perhaps with a default of 1000) would probably solve clark's use case.

          Also, if I'm reading this right, it will ignore the broker ID if it's set in the config but out of the reserved range (set to -1). It will then create a generated broker ID to overwrite the one in the config. It might be nice to have the broker error out on startup if a broker.id is set in the config, but is out of range. A new broker ID should only be generated if an id isn't specified in the config AND the meta.properties doesn't exist (otherwise the config file wouldn't match the actual broker ID). Edit: nevermind, my Scala reading skills aren't up to par. This isn't the case here.

          Show
          Jon Bringhurst added a comment - - edited Clark Haskins , Sriharsha Chintalapani , having ReservedBrokerIdMaxValue as a configurable option (perhaps with a default of 1000) would probably solve clark's use case. Also, if I'm reading this right, it will ignore the broker ID if it's set in the config but out of the reserved range (set to -1). It will then create a generated broker ID to overwrite the one in the config. It might be nice to have the broker error out on startup if a broker.id is set in the config, but is out of range. A new broker ID should only be generated if an id isn't specified in the config AND the meta.properties doesn't exist (otherwise the config file wouldn't match the actual broker ID). Edit: nevermind, my Scala reading skills aren't up to par. This isn't the case here.
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Joe Stein added a comment - - edited

          This sounds like a very cool and VERY useful feature. Excited to use it myself often.

          I know of a few (>10) different clusters that not only use varying sized numbers for their broker.id but do so in what is a seemingly random (but not really when you think about it) way.

          so in a cluster there may be broker.id that is 1721632 and another 172164875 and another 172162240 . Making your brokers by replacing "." in chef/puppet/salt/ansemble/etc type scripts and sometimes folks get more fancy just doing 2288, 2388, 17, 177 (where just the last two octets get used and "." is replaced).

          I am not saying I agree with this approach and I actively advocate away from doing this but in some/many scenarios it is the best/only way to automate their deploys for how things are setup. It is also what seems to make sense when folks are building their automation scripts since they have no other option without doing more than they should be expected to-do (and the IP replace "." is so obvious to them, and it is).

          So, for folks in these cases they would just pick the upper bound to be, lets say 17216255256 and then it would auto assign from there?

          Is there some better way to go about this where you might have a start increment and and some exclusion list? or a way to see broker.id already had that and not use it? I think a lot of folks would like to get having broker id be more continious and be easier to communicate but the desire to automate everything will outweigh that. We could give them some sanity back with brokers 1,2,3,4,5,6,7,8,9,10,11,12 for a 12 node cluster.

          not crucial and you may have already accounted for the scenario I brought up, but wanted to bring it up as a real use case for how people automate things.

          it might be better for folks to manually migrate their scripts but not sure they would do it and if they did would have to decommission brokers which in a prod environment could take a few weeks/months. If we let them start at 1 and exclude what they have then they can do it one at a time. After taking down the first broker and bring it back up (empty) it is broker.id=1, and so on (and if they have a 5 they don't have to take it down), etc.

          For new clusters this is a slam dunk and wouldn't want to hold up the feature for existing users that have already decided a work around as I don't know what the intent of this was or not. Some folks might not change knowing broker.id=17216520 sometimes is nice you just login to that box but talking about broker 17216520 over and over again is a pita.

          Show
          Joe Stein added a comment - - edited This sounds like a very cool and VERY useful feature. Excited to use it myself often. I know of a few (>10) different clusters that not only use varying sized numbers for their broker.id but do so in what is a seemingly random (but not really when you think about it) way. so in a cluster there may be broker.id that is 1721632 and another 172164875 and another 172162240 . Making your brokers by replacing "." in chef/puppet/salt/ansemble/etc type scripts and sometimes folks get more fancy just doing 2288, 2388, 17, 177 (where just the last two octets get used and "." is replaced). I am not saying I agree with this approach and I actively advocate away from doing this but in some/many scenarios it is the best/only way to automate their deploys for how things are setup. It is also what seems to make sense when folks are building their automation scripts since they have no other option without doing more than they should be expected to-do (and the IP replace "." is so obvious to them, and it is). So, for folks in these cases they would just pick the upper bound to be, lets say 17216255256 and then it would auto assign from there? Is there some better way to go about this where you might have a start increment and and some exclusion list? or a way to see broker.id already had that and not use it? I think a lot of folks would like to get having broker id be more continious and be easier to communicate but the desire to automate everything will outweigh that. We could give them some sanity back with brokers 1,2,3,4,5,6,7,8,9,10,11,12 for a 12 node cluster. not crucial and you may have already accounted for the scenario I brought up, but wanted to bring it up as a real use case for how people automate things. it might be better for folks to manually migrate their scripts but not sure they would do it and if they did would have to decommission brokers which in a prod environment could take a few weeks/months. If we let them start at 1 and exclude what they have then they can do it one at a time. After taking down the first broker and bring it back up (empty) it is broker.id=1, and so on (and if they have a 5 they don't have to take it down), etc. For new clusters this is a slam dunk and wouldn't want to hold up the feature for existing users that have already decided a work around as I don't know what the intent of this was or not. Some folks might not change knowing broker.id=17216520 sometimes is nice you just login to that box but talking about broker 17216520 over and over again is a pita.
          Hide
          Sriharsha Chintalapani added a comment -

          Joe Stein Thanks for the comments. Currently the approach allows users to specify MaxBrokerId and kafka starts generating a sequence from MaxBrokerId + 1. If user specifies brokerId , kafka uses that instead of generating one from zookeeper. In the current implementation we expect user configured broker.id to be less than MaxBrokerId.
          The tricky case here is if the user have a cluster where some of the brokers have broker.id present in their config and others don't. In this case we use MaxBrokerId (defaults to 1000) and start generating a sequenceId from there.
          The issue with brokerId exception list is we do not know from which point to generate a sequenceId.
          In your example " in a cluster there may be broker.id that is 1721632 and another 172164875 and another 172162240 "
          and user added another broker to the cluster without broker.id in config. If the user specifies that MaxReserveBrokerId is 172164875 than we can assign +1 to that new broker. If we take the approach of looking at brokerIds that exists in the cluster and generate a new one that doesn't exist or taken by a broker already(exclusion list as you suggested) than the problem would be how do we know what are all the broker.ids existed in the cluster one way to look at is zookeeper but a broker registers at zk after it starts.
          For ex: if a broker1 starts with 1721632 and broker2 starts with no broker.id than we generate 1721633 from zk and there is no guarantee that another broker has this same id in their config. Of course we can have a exclusion list of brokerids in a config file but that feels cumbersome for the user to list out all the brokerids.

          Show
          Sriharsha Chintalapani added a comment - Joe Stein Thanks for the comments. Currently the approach allows users to specify MaxBrokerId and kafka starts generating a sequence from MaxBrokerId + 1. If user specifies brokerId , kafka uses that instead of generating one from zookeeper. In the current implementation we expect user configured broker.id to be less than MaxBrokerId. The tricky case here is if the user have a cluster where some of the brokers have broker.id present in their config and others don't. In this case we use MaxBrokerId (defaults to 1000) and start generating a sequenceId from there. The issue with brokerId exception list is we do not know from which point to generate a sequenceId. In your example " in a cluster there may be broker.id that is 1721632 and another 172164875 and another 172162240 " and user added another broker to the cluster without broker.id in config. If the user specifies that MaxReserveBrokerId is 172164875 than we can assign +1 to that new broker. If we take the approach of looking at brokerIds that exists in the cluster and generate a new one that doesn't exist or taken by a broker already(exclusion list as you suggested) than the problem would be how do we know what are all the broker.ids existed in the cluster one way to look at is zookeeper but a broker registers at zk after it starts. For ex: if a broker1 starts with 1721632 and broker2 starts with no broker.id than we generate 1721633 from zk and there is no guarantee that another broker has this same id in their config. Of course we can have a exclusion list of brokerids in a config file but that feels cumbersome for the user to list out all the brokerids.
          Hide
          Joe Stein added a comment - - edited

          Sriharsha Chintalapani I understand. What I am struggling with is this for new users or existing users? For existing users what I have seen them do (at least a dozen times now) is to avoid the conflict of broker.id in the properties file (like you are saying) is to take the IP address, strip out the "." and use that (since IP is unique that is what they go with). They do this in their automation scripts. I have to imagine it is larger than I have seen as they all did this independently of each other as an intuitive way to make unique broker.id.

          Show
          Joe Stein added a comment - - edited Sriharsha Chintalapani I understand. What I am struggling with is this for new users or existing users? For existing users what I have seen them do (at least a dozen times now) is to avoid the conflict of broker.id in the properties file (like you are saying) is to take the IP address, strip out the "." and use that (since IP is unique that is what they go with). They do this in their automation scripts. I have to imagine it is larger than I have seen as they all did this independently of each other as an intuitive way to make unique broker.id.
          Hide
          Sriharsha Chintalapani added a comment -

          Jay Kreps Jun Rao Neha Narkhede Can you please review the last patch I sent. Thanks.

          Show
          Sriharsha Chintalapani added a comment - Jay Kreps Jun Rao Neha Narkhede Can you please review the last patch I sent. Thanks.
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Jay Kreps Jun Rao Neha Narkhede Can you please review the latest patch. Thanks.

          Show
          Sriharsha Chintalapani added a comment - Jay Kreps Jun Rao Neha Narkhede Can you please review the latest patch. Thanks.
          Hide
          Neha Narkhede added a comment -

          In an attempt to keep up with reviews, assigning to Jay Kreps since he has most context on the patch. Feel free to reassign

          Show
          Neha Narkhede added a comment - In an attempt to keep up with reviews, assigning to Jay Kreps since he has most context on the patch. Feel free to reassign
          Hide
          Sriharsha Chintalapani added a comment - - edited

          Neha Narkhede Jun Rao Jay Kreps Guozhang Wang Hi All, I am trying to get this patch reviewed. Please let me know if there is anything that need to be fixed with this patch. Thank you.

          Show
          Sriharsha Chintalapani added a comment - - edited Neha Narkhede Jun Rao Jay Kreps Guozhang Wang Hi All, I am trying to get this patch reviewed. Please let me know if there is anything that need to be fixed with this patch. Thank you.
          Hide
          Otis Gospodnetic added a comment -

          sriharsha chintalapani - it looks like a number of people would really like to use IPs for broker.id. There is a lot of interest in having that. Please see this thread: http://search-hadoop.com/m/4TaT4dTPKi1
          Do you think this is something you could add to this patch, maybe as another broker ID assignment scheme/policy?

          Show
          Otis Gospodnetic added a comment - sriharsha chintalapani - it looks like a number of people would really like to use IPs for broker.id. There is a lot of interest in having that. Please see this thread: http://search-hadoop.com/m/4TaT4dTPKi1 Do you think this is something you could add to this patch, maybe as another broker ID assignment scheme/policy?
          Hide
          Sriharsha Chintalapani added a comment -

          Otis Gospodnetic this patch pending the 0.8.2 release. Since the we have 0.8.2 branched out I'll upmerge this patch and add a configurable option to server.properties where if user prefers IP based broker Id we will generate that or use the seq id that I currently have as a default.
          Jay Kreps Neha Narkhede Jun Rao please let me know if the above approach looks good to you. Thanks.

          Show
          Sriharsha Chintalapani added a comment - Otis Gospodnetic this patch pending the 0.8.2 release. Since the we have 0.8.2 branched out I'll upmerge this patch and add a configurable option to server.properties where if user prefers IP based broker Id we will generate that or use the seq id that I currently have as a default. Jay Kreps Neha Narkhede Jun Rao please let me know if the above approach looks good to you. Thanks.
          Hide
          Otis Gospodnetic added a comment -

          this patch pending the 0.8.2 release

          Note this issue doesn't have Fix Version set to 0.8.2. Maybe that's what you want? +1 from me – it looks like a number of people would like to see this issue resolved!

          Show
          Otis Gospodnetic added a comment - this patch pending the 0.8.2 release Note this issue doesn't have Fix Version set to 0.8.2. Maybe that's what you want? +1 from me – it looks like a number of people would like to see this issue resolved!
          Hide
          Sriharsha Chintalapani added a comment -

          Otis Gospodnetic Sorry I meant pending review after 0.8.2 release. Not planned for 0.8.2 release. I'll update the patch with ip as brokerid option.

          Show
          Sriharsha Chintalapani added a comment - Otis Gospodnetic Sorry I meant pending review after 0.8.2 release. Not planned for 0.8.2 release. I'll update the patch with ip as brokerid option.
          Hide
          Neha Narkhede added a comment -

          Sorry for the late review. Expect to get to this shortly.

          Show
          Neha Narkhede added a comment - Sorry for the late review. Expect to get to this shortly.
          Hide
          Neha Narkhede added a comment -

          Sriharsha Chintalapani Would you mind rebasing your patch? Can help you review it in the next few days.

          Show
          Neha Narkhede added a comment - Sriharsha Chintalapani Would you mind rebasing your patch? Can help you review it in the next few days.
          Hide
          Sriharsha Chintalapani added a comment -

          Neha Narkhede will implement the requested ip based broker id as an option and send an updated patch. Try to get it done this week.

          Show
          Sriharsha Chintalapani added a comment - Neha Narkhede will implement the requested ip based broker id as an option and send an updated patch. Try to get it done this week.
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Neha Narkhede Joe Stein Otis Gospodnetic
          Please take a look at the latest patch. I added broker.id.policy as a configurable option with "ip" and "sequence" .
          I've few questions regarding "ip" based broker.id.
          currently the patch expects /etc/hosts to be properly configured with an ip associated with the hostname.
          1) should we ignore loopback ip
          i) if we don't consider loopback ip than on a single node machine with /etc/hosts is not being configured properly it will thrown an error
          ii) if we consider this than there is a same broker id being generate on multiple hosts or multiple brokers on a single host ( this could be the case in non loopback ips too)
          2) broker.id is Int and there is a possibility of throwing a NumberFormatException if the ip doesn't fit in Int.MaxValue
          3) If a host contains multiple network interfaces which ip should we pick up. Current implementation takes whatever configured for the hostname in /etc/hosts which seems ok to me .
          Please let me know your thoughts on the above. Thanks.

          Show
          Sriharsha Chintalapani added a comment - Neha Narkhede Joe Stein Otis Gospodnetic Please take a look at the latest patch. I added broker.id.policy as a configurable option with "ip" and "sequence" . I've few questions regarding "ip" based broker.id. currently the patch expects /etc/hosts to be properly configured with an ip associated with the hostname. 1) should we ignore loopback ip i) if we don't consider loopback ip than on a single node machine with /etc/hosts is not being configured properly it will thrown an error ii) if we consider this than there is a same broker id being generate on multiple hosts or multiple brokers on a single host ( this could be the case in non loopback ips too) 2) broker.id is Int and there is a possibility of throwing a NumberFormatException if the ip doesn't fit in Int.MaxValue 3) If a host contains multiple network interfaces which ip should we pick up. Current implementation takes whatever configured for the hostname in /etc/hosts which seems ok to me . Please let me know your thoughts on the above. Thanks.
          Hide
          Neha Narkhede added a comment -

          Sriharsha Chintalapani Reviewed your latest patch. Posted review comments on the rb.

          Show
          Neha Narkhede added a comment - Sriharsha Chintalapani Reviewed your latest patch. Posted review comments on the rb.
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Neha Narkhede Thanks for the review. Updated the patch please take a look .

          Show
          Sriharsha Chintalapani added a comment - Neha Narkhede Thanks for the review. Updated the patch please take a look .
          Hide
          Sriharsha Chintalapani added a comment -

          Neha Narkhede Can you please take a look at the patch and also reply to your comments. Thanks.

          Show
          Sriharsha Chintalapani added a comment - Neha Narkhede Can you please take a look at the patch and also reply to your comments. Thanks.
          Hide
          Neha Narkhede added a comment -

          Thanks for the updated patch Sriharsha Chintalapani. Almost there. Left a few more cleanup comments.

          Show
          Neha Narkhede added a comment - Thanks for the updated patch Sriharsha Chintalapani . Almost there. Left a few more cleanup comments.
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Thanks for the review Neha Narkhede. I updated the patch , please take a look when you get a chance . Thanks.

          Show
          Sriharsha Chintalapani added a comment - Thanks for the review Neha Narkhede . I updated the patch , please take a look when you get a chance . Thanks.
          Hide
          Neha Narkhede added a comment -

          Sriharsha Chintalapani There are still questions around the broker metadata file serialization. I've reviewed the latest patch and left my comments on the rb.

          Show
          Neha Narkhede added a comment - Sriharsha Chintalapani There are still questions around the broker metadata file serialization. I've reviewed the latest patch and left my comments on the rb.
          Hide
          Sriharsha Chintalapani added a comment -

          Thanks Neha Narkhede . I'll send an updated patch.

          Show
          Sriharsha Chintalapani added a comment - Thanks Neha Narkhede . I'll send an updated patch.
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Neha Narkhede updated patch addresses 2 of your comments and I've replied to your other comments. Please check and let me know. Thank for the review.

          Show
          Sriharsha Chintalapani added a comment - Neha Narkhede updated patch addresses 2 of your comments and I've replied to your other comments. Please check and let me know. Thank for the review.
          Hide
          Neha Narkhede added a comment -

          Sriharsha Chintalapani Thanks for your efforts in getting this patch in shape. Pushed to trunk

          Show
          Neha Narkhede added a comment - Sriharsha Chintalapani Thanks for your efforts in getting this patch in shape. Pushed to trunk
          Hide
          Sriharsha Chintalapani added a comment -

          Updated reviewboard https://reviews.apache.org/r/23702/diff/
          against branch origin/trunk

          Show
          Sriharsha Chintalapani added a comment - Updated reviewboard https://reviews.apache.org/r/23702/diff/ against branch origin/trunk
          Hide
          Alex Etling added a comment -

          Hello Kafka Geniuses,
          I have a question regarding this PR. Some context:
          We have started playing around with kafka 0.8.2.1 to build a data pipeline at the company I work at. Initially, to get autoscaling with AWS working, we were mapping the IP of our boxes to our broker id. For a while everything was good. Today I realized (please correct me if I am wrong) that kafka assigns the replicas to a topic at topic creation time. These replicas are not modified later unless you specifically rebalance the cluster(this is different than ISR which can go from 0 servers to the set of replicas). This leads to an interesting question on how to cycle in new boxes. The easiest way seems to be to copy all data from one box to another, kill the old box and start the new box with the same broker.id. This is not really easy when you do a direct mapping of IP -> broker.id.
          So now we come to this Jira ticket. I was wondering if you could enumerate for me how this auto-assign node id would deal with the cycling of a box. If a bring down a box that was auto-assigned a broker.id of X and bring back up a new box, what will happen. Will that new box have broker.id X as well? What if I bring down two boxes with broker.id X and broker.id Y, what is the broker.id of the new box i spin up.

          Thanks for the help,
          Alex

          Show
          Alex Etling added a comment - Hello Kafka Geniuses, I have a question regarding this PR. Some context: We have started playing around with kafka 0.8.2.1 to build a data pipeline at the company I work at. Initially, to get autoscaling with AWS working, we were mapping the IP of our boxes to our broker id. For a while everything was good. Today I realized (please correct me if I am wrong) that kafka assigns the replicas to a topic at topic creation time. These replicas are not modified later unless you specifically rebalance the cluster(this is different than ISR which can go from 0 servers to the set of replicas). This leads to an interesting question on how to cycle in new boxes. The easiest way seems to be to copy all data from one box to another, kill the old box and start the new box with the same broker.id. This is not really easy when you do a direct mapping of IP -> broker.id. So now we come to this Jira ticket. I was wondering if you could enumerate for me how this auto-assign node id would deal with the cycling of a box. If a bring down a box that was auto-assigned a broker.id of X and bring back up a new box, what will happen. Will that new box have broker.id X as well? What if I bring down two boxes with broker.id X and broker.id Y, what is the broker.id of the new box i spin up. Thanks for the help, Alex
          Hide
          Sriharsha Chintalapani added a comment -

          Alex Etling Here auto-assign node id is one-time op. Currently users have to declare a broker.id in server.properties and this is unique per broker in a cluster.
          Whats this JIRA addressed is to allow users to not to declare broker.id instead kafka when it startsup acquire a sequnce id from zookeeper writes it into meta.properties to use as broker.id
          once this is generated and written meta.properties it won't generate a new id. So the problem you are trying solve is not addressed by this JIRA it just allows users not to worry about declaring a broker.id in server.properties.

          Show
          Sriharsha Chintalapani added a comment - Alex Etling Here auto-assign node id is one-time op. Currently users have to declare a broker.id in server.properties and this is unique per broker in a cluster. Whats this JIRA addressed is to allow users to not to declare broker.id instead kafka when it startsup acquire a sequnce id from zookeeper writes it into meta.properties to use as broker.id once this is generated and written meta.properties it won't generate a new id. So the problem you are trying solve is not addressed by this JIRA it just allows users not to worry about declaring a broker.id in server.properties.
          Hide
          Alex Etling added a comment -

          Thank you for the help! I appreciate it.

          Show
          Alex Etling added a comment - Thank you for the help! I appreciate it.
          Hide
          Adrian Muraru added a comment -

          Neha Narkhede For some reason this patch was not applied correctly in trunk branch, I see an .orig file in the patch here:
          https://github.com/apache/kafka/commit/b1b80860a01cc378cfada3549a3480f0773c3ff8#diff-d0716898c8daed9887ef83500ee0e16e

          Show
          Adrian Muraru added a comment - Neha Narkhede For some reason this patch was not applied correctly in trunk branch, I see an .orig file in the patch here: https://github.com/apache/kafka/commit/b1b80860a01cc378cfada3549a3480f0773c3ff8#diff-d0716898c8daed9887ef83500ee0e16e
          Hide
          Sriharsha Chintalapani added a comment -

          Adrian Muraru Patch is already merged into trunk.

          Show
          Sriharsha Chintalapani added a comment - Adrian Muraru Patch is already merged into trunk.
          Hide
          Adrian Muraru added a comment -

          Saw it, that's why I raised the flag here, the patch in trunk seems broken with the .orig file added. A new jira issue might be needed to fix it

          Show
          Adrian Muraru added a comment - Saw it, that's why I raised the flag here, the patch in trunk seems broken with the .orig file added. A new jira issue might be needed to fix it
          Hide
          Sriharsha Chintalapani added a comment -

          Adrian Muraru I am not sure where you are seeing .orig file in the trunk. Can you paste the link to that file from here https://github.com/apache/kafka

          Show
          Sriharsha Chintalapani added a comment - Adrian Muraru I am not sure where you are seeing .orig file in the trunk. Can you paste the link to that file from here https://github.com/apache/kafka
          Hide
          Adrian Muraru added a comment -

          sriharsha chintalapani I see KAFKA-1973 already fixed that, nw

          Show
          Adrian Muraru added a comment - sriharsha chintalapani I see KAFKA-1973 already fixed that, nw

            People

            • Assignee:
              Sriharsha Chintalapani
              Reporter:
              Jay Kreps
              Reviewer:
              Neha Narkhede
            • Votes:
              3 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development