Another quick comment: the KIP states that having multiple interfaces imply that the logic must be in 2 places. That is not true because the same class can implement multiple interfaces (this aspect was considered when we decided to introduce policies incrementally).
The main reason why I think the original approach doesn't work well is that there is no direct mapping between an operation and the policy. That is, we initially thought we would have create/alter/delete topics, but that didn't work out as the alter case is better served by multiple request types. Given that, it's a bit awkward to maintain the original approach and a policy for topic management seemed easier to understand. On that note, would `TopicManagementPolicy` be a better name? Looking at the updated KIP, I notice that we actually have a TopicDeletionPolicy with a separate config. That seems to be a halfway house. Not sure about that. Ismael On Wed, Sep 27, 2017 at 10:47 AM, Tom Bentley <t.j.bent...@gmail.com> wrote: > I have updated the KIP to add a common policy interface for topic and > message deletion. This included pulling ClusterState and TopicState > interfaces up to the top level so that they can be shared between the two > policies. > > Cheers, > > Tom > > On 26 September 2017 at 18:09, Edoardo Comar <eco...@uk.ibm.com> wrote: > > > Thanks Tom, > > In my original KIP-170 I mentioned that the method > > > > public Map<String, Integer> topicsPartitionCount(); > > > > was just a starting point for a general purpose ClusterState > > as it happened to be exactly the info we needed for our policy > > implementation :-) > > it definitely doesn't feel general purpose enough. > > > > what about > > > > interface ClusterState { > > public TopicState topicState(String topicName); > > public Set<String> topics(); > > } > > > > I think that the implementation of ClusterState that the server will pass > > to the policy.validate method > > would just lazily tap into MetadataCache. No need for big upfront > > allocations. > > > > ciao, > > Edo > > -------------------------------------------------- > > > > Edoardo Comar > > > > IBM Message Hub > > > > IBM UK Ltd, Hursley Park, SO21 2JN > > > > > > > > From: Tom Bentley <t.j.bent...@gmail.com> > > To: dev@kafka.apache.org > > Date: 26/09/2017 17:39 > > Subject: Re: [DISCUSS] KIP-201: Rationalising Policy interfaces > > > > > > > > Hi Edoardo, > > > > > > what about a single method in ClusterState > > > > > > interface ClusterState { > > > public Map<String,TopicState> topicsState(); > > > > > > } > > > > > > which could return a read-only snapshot of the cluster metadata ? > > > > > > > Sure that would work too. A concern with that is that we end up > allocating > > a potentially rather large amount for the Map and the collections present > > in the TopicStates in order to provide the snapshot. The caller might > only > > be interested in one item from the TopicState for one topic in the map. > > Accessing this information via methods means the caller only pays for > what > > they use. > > > > Cheers, > > > > Tom > > > > > > > > Unless stated otherwise above: > > IBM United Kingdom Limited - Registered in England and Wales with number > > 741598. > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 > 3AU > > >