Responding to a few of the other comments: it seems that you propagated > the topic deletion marker by having the replicaManager read from ZK > directly. It seems that it would be simpler/consistent if the controller > propagates that information directly through UpdateMetaRequest.
I was told that I should not try and modify controller logic with KIP-4 changes. It was indicated that a larger controller rewrite and testing was planned and those changes should be considered then. Since marking a topic for deletion doesn't flow through the controller and therefore the UpdateMetadataRequest, it would take quite a bit of change. We would need to trigger a new UpdateMetadataRequest every time a new topic is marked for deletion. Instead I added a listener to maintain a cache of the topic deletion znodes in the ReplicaManager where the existing UpdateMetadataRequests are handled. This would make it easy to swap out later once the data is a part of that request and have minimal impact in the mean time. > Could you add a description on how controller id will be used in the > client? I will add it to the wiki. Today metrics are the only way to access this piece of data. It is useful information about the cluster for many reasons. Having programatic access to identify the controller is helpful for automation. For example, It can be used during rolling restart logic to shutdown the controller last to prevent multiple fail overs. Beyond automation, it can be leveraged in KIP-4 to route admin requests to the controller broker. We had a weird semantic in version 0 of MetadataRequest. If a replica is > not live, but the leader is live, we return an > error ReplicaNotAvailableException in the partition metadata. This makes it > a bit confusing for the client to parse since it has to first check whether > leader is available or not before error code checking. We were thinking of > changing that behavior the next time we bump up the version of > MetadataRequest. > Now that time has come, could you include that in the proposal? I am not sure I completely follow the issue and requested change. Could you point me to the discussion? Thank you, Grant On Sun, Apr 3, 2016 at 8:09 PM, Grant Henke <ghe...@cloudera.com> wrote: > Hi Jun and Ismael, > > Initially I had 2 booleans used to indicate if a topic was internal and if > a topic was marked for deletion. To save space on large deployments, Ismael > suggested I break out the internal topics and deleted topics into their own > lists. The idea was that instead of 2 bytes added per topic, in the general > case the lists would be empty. Even in those lists I still only return > topics that were requested. In fact on the client side they are just > utilized to translate back to booleans. I do prefer the booleans from an > expressiveness standpoint but was not strongly opinionated on the > structure. > > Thank you, > Grant > > On Sun, Apr 3, 2016 at 8:00 PM, Ismael Juma <ism...@juma.me.uk> wrote: > >> Hi Jun, >> >> A couple of comments inline. >> >> On Sun, Apr 3, 2016 at 5:55 PM, Jun Rao <j...@confluent.io> wrote: >> >> > 1. It seems a bit weird to return just a list of internal topics w/o the >> > corresponding metadata. It also seems a bit weird to return the internal >> > topics even if the client doesn't ask for it. >> >> >> Good point. >> >> >> > Would it be better to just >> > add a flag in topic_metadata to indicate whether it's an internal topic >> or >> > not, and only include the internal topics when thy are asked (or all >> topics >> > are requested) for? >> > >> >> The disadvantage of this is that we are adding one byte per topic even >> though we have a very small number of internal topics (currently a single >> internal topic). It seems a bit wasteful and particularly so when using >> regex subscriptions (since we have to retrieve all topics in that case). >> >> 2. A similar comment on topics_marked_for_deletion. Would it be better to >> > only return them when asked for and just return a new TopicDeleted error >> > code in topic_metadata? >> >> >> I agree that this seems better. >> >> Ismael >> > > > > -- > Grant Henke > Software Engineer | Cloudera > gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke > -- Grant Henke Software Engineer | Cloudera gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
