Hey Ismael, Yes, user has the option to manually set the cluster.id by directly setting the znode. But the KIP doesn't provide script for doing this. Unlike reading cluster.id from broker, this approach doesn't allow you to persist the cluster.id in case of cluster migration and znode deletion.
In case of zookeeper migration or znode deletion, you can read the cluster.id from previous log. But log is not a good place to persist anything you need because it is not persisted in any repository. Reading a config value from log for recovery also appears adhoc. I agree that in terms of implementation you can add the cluster.id config value in a future KIP without affecting this KIP. But why couldn't we do it altogether if it is not too hard and if this adds value on top of existing proposal? I think we are encouraged to think beyond the existing design of the KIP to see if it can serve its target use-case better, if the improvement doesn't require too much additional work. Readability and ability to persist cluster.id in config seems very useful to the use-case described in the KIP. I am just trying to contribute here by providing the review. If you think the benefits above is trivial and doesn't have to be included in this KIP, I am OK with that. Thanks for all the discussion. Dong On Mon, Sep 5, 2016 at 2:41 PM, Ismael Juma <ism...@juma.me.uk> wrote: > Hi Dong, > > A few clarifications below. > > On Mon, Sep 5, 2016 at 7:21 PM, Dong Lin <lindon...@gmail.com> wrote: > > > > I think you are saying that we can stop our discussion and follow simply > > take a vote where the majority decides. I don't think this is a good way > to > > find the best design for a KIP and the discussion seems to be useless. > > > I didn't mean that we should stop the discussion. I meant that if we can't > reach agreement, it would make sense to try and get feedback from a wider > group in the interest of making progress. As you said below, a large number > of messages were exchanged by just 3 people. Even if we were in agreement, > we would not be able to pass the KIP by ourselves. Also, I would certainly > not classify the discussion as useless. The alternative you are proposing > has evolved as part of the discussion and the KIP has been updated a couple > of times. > > > > My argument for the 2nd approach is that it additionally allows user to > use > > human readable cluster.id if they want to do it. > > > As discussed previously, you can also set the cluster id manually with the > existing KIP by setting the relevant znode. One could describe it as an > intentionally expert-level option instead of the more accessible broker > config (the broker level config does mean that you have to set it for every > broker, but that's probably fine since it's also the case for a few other > configs). > > There are two major concern with this KIP: > > > > - The KIP says that it is a requirement for cluster.id to be immutable. > > Ismael suggests that the reading cluster.id from config doesn't meet > this > > requirement. However, the current approach described in the KIP doesn't > > satisfy this requirement either. If user deletes the znode that stores > > cluster.id, either intentionally or by mistake, the cluster.id is lost > > forever. The KIP should be updated to reflect this and this requirement > can > > not be used in the comparison between the different approaches. > > > > I will update the KIP to make this point clearer. A couple of comments: > > 1. Immutability is a goal and the recommended usage, but you can work > around it via the expert-level znode update. You have much bigger problems > if you delete random znodes by mistake to be honest. > 2. The cluster id is not lost forever since it can be trivially recovered > from the logs. > > - One of the argument against reading cluster.id from config is that > "unique > > and immutable auto-generated id + changeable human-readable name is a > > better overall solution". Sumit describes the long term plan to use > > readable tags as well. However, the KIP doesn't describe the design of > > using this readable name/tags. The KIP needs to provide more information > > about this plan if this is used to argue for the existing approach > against > > an alternative. > > > > I am not sure why this is so. After all, your suggestion of a broker config > is an addition to what is being proposed in the KIP. Why can't we evaluate > your suggestion at the same time as the resource tags proposal? What we are > proposing in the current KIP is a foundation that is needed in both cases. > What am I missing? > > Ismael >
