A couple more comments:

1. "If this KIP is accepted for Kafka 1.1.0 this removal could happen in
Kafka 1.2.0 or a later release." -> we only remove code in major releases.
So, if it's deprecated in 1.1.0, it would be removed in 2.0.0.

2. Deleting all messages in a topic is not really the same as deleting a
topic. The latter will cause consumers and producers to error out while the
former will not. It would be good to motivate the need for the delete
records policy more.

Ismael

On Wed, Sep 27, 2017 at 11:12 AM, Ismael Juma <ism...@juma.me.uk> wrote:

> 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
>> >
>>
>
>

Reply via email to