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