Hi Flavio, Thanks for reviewing the KIP. Comments inline.
On Sat, Sep 3, 2016 at 4:48 PM, Flavio Junqueira <f...@apache.org> wrote: > Thanks for the KIP, Ismael, looks great. I have just a couple of comments > and questions: > > - I assume the znode containing the cluster id is persistent. Is there > ever the need to delete that znode to force a new instance id, and if so, > how is it expected to happen? I'm thinking about a scenario in which I have > a cluster, I wipe out intentionally to reset my state, and I want to start > with a fresh instance of the cluster. In such a case, it should be as > simple as deleting the znode manually, so I'm just wondering if this is > what we are going to expect from users. The alternative is to assume that > the zookeeper ensemble is also going to be wiped out in this case, or at > least the corresponding sub-tree. > Yes, my expectation is that if you want to start from scratch, you'd delete the corresponding sub-tree (which will include the cluster id). > - I'm not actually suggesting that we do things manually, but I can see > some benefit in manually creating the cluster id znode and have brokers > only check that the znode is there before starting. > Yes, I agree that there are some benefits in doing it this way. The reason why we do it automatically is we didn't want to add an additional step for people who are starting a Kafka cluster. > - One of the messages mentions changing names, which is a good point. If > it isn't necessary for correctness that all brokers have either no id or > the same id, then I'd think it is fine to just watch that znode for > changes. Brokers that are connect to zk will receive eventually a > notification. But, you point out that changing the name for a working > cluster isn't necessary a desirable feature, so this might not be needed. > Indeed, the idea is that this id should never change for a working cluster (i.e. it should be unique and immutable). There are exceptions like the scenario you pointed out: a user wants to intentionally reset the state and start a fresh instance of the cluster (I think one could look at this as starting a new cluster though). Ismael
