You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Tom Bentley <tb...@redhat.com> on 2019/08/12 09:52:24 UTC
Re: [DISCUSS] KIP-201: Rationalising Policy interfaces
Hi folks,
As far as I can see the motivation for KIP-201 is still valid, and as far
as I'm aware the changes I made to the KIP back in April addressed the
previous comments. Since the issue still needs to be addressed I intend to
start another vote thread in the near future, but before I do I thought I'd
check whether anyone has any more comments. So please let me know any
feedback for this KIP.
Many thanks,
Tom
On Fri, Apr 12, 2019 at 10:46 AM Tom Bentley <tb...@redhat.com> wrote:
> Hi Rajini,
>
> I've made a number of changes to the KIP.
>
> 1. I've added RequestedTopicState.requestedConfigs(). This is obviously
> unrelated to supporting alter broker, but I think it goes some way to
> addressing one of the points Anna made last year.
> Anna, wdyt?
>
> 2. I've added BrokerState, RequestedBrokerState and
> BrokerManagementPolicy. These are largely similar to the interfaces for
> topic management, but the lifecycle of a BrokerManagementPolicy needs to be
> different.
>
> That's because a BrokerManagementPolicy ought to be Configurable with the
> broker config, but obviously the broker config can change. Because a
> cluster-scoped config might be changed via a different broker we need to
> hook into the Zookeeper change notification on the broker configs to
> instantiate a new BrokerManagementPolicy when broker policy changes. We'd
> need to cope with policy implementation change happening concurrently with
> policy enforcement.
> And technically there's a race here: Sending changes to cluster-scoped
> configs to multiple brokers could result in non-deterministic policy
> enforcement.
>
> One way to avoid that would be to require changes to cluster-scoped
> configs to be sent to the controller.
> This complexity is annoying because it seems likely that many policy
> implementations won't _actually_ depend on the broker config.
>
> Thoughts?
>
> Kind regards,
>
> Tom
>
> On Wed, Apr 10, 2019 at 9:48 AM Rajini Sivaram <ra...@gmail.com>
> wrote:
>
>> Thanks Tom.
>>
>> Once you have updated the KIP to support broker config updates, it may be
>> good to start a new vote thread since the other one is quite old and
>> perhaps the KIP has changed since then.
>>
>>
>> On Wed, Apr 10, 2019 at 3:58 AM Tom Bentley <tb...@redhat.com> wrote:
>>
>> > Hi Rajini,
>> >
>> > I'd be happy to do that. I'll try to get it done in the next few days.
>> >
>> > Although there's been quite a lot of interest this, the vote thread
>> never
>> > got any binding +1, so it's been stuck in limbo for a long time. It
>> would
>> > be great to get this moving again.
>> >
>> > Kind regards,
>> >
>> > Tom
>> >
>> > On Tue, Apr 9, 2019 at 3:04 PM Rajini Sivaram <ra...@gmail.com>
>> > wrote:
>> >
>> > > Hi Tom,
>> > >
>> > > Are you planning to extend this KIP to also include dynamic broker
>> config
>> > > update (currently covered under AlterConfigPolicy)?
>> > >
>> > > May be worth sending another note to make progress on this KIP since
>> it
>> > has
>> > > been around a while and reading through the threads, it looks like
>> there
>> > > has been a lot of interest in it.
>> > >
>> > > Thank you,
>> > >
>> > > Rajini
>> > >
>> > >
>> > > On Wed, Jan 9, 2019 at 11:25 AM Tom Bentley <t....@gmail.com>
>> > wrote:
>> > >
>> > > > Hi Anna and Mickael,
>> > > >
>> > > > Anna, did you have any comments about the points I made?
>> > > >
>> > > > Mickael, we really need the vote to be passed before there's even
>> any
>> > > work
>> > > > to do. With the exception of Ismael, the KIP didn't seem to get the
>> > > > attention of any of the other committers.
>> > > >
>> > > > Kind regards,
>> > > >
>> > > > Tom
>> > > >
>> > > > On Thu, 13 Dec 2018 at 18:11, Tom Bentley <t....@gmail.com>
>> > wrote:
>> > > >
>> > > > > Hi Anna,
>> > > > >
>> > > > > Firstly, let me apologise again about having missed your previous
>> > > emails
>> > > > > about this.
>> > > > >
>> > > > > Thank you for the feedback. You raise some valid points about
>> > > ambiguity.
>> > > > > The problem with pulling the metadata into CreateTopicRequest and
>> > > > > AlterTopicRequest is that you lose the benefit of being able to
>> eaily
>> > > > write
>> > > > > a common policy across creation and alter cases. For example, with
>> > the
>> > > > > proposed design the policy maker could write code like this
>> (forgive
>> > my
>> > > > > pseudo-Java)
>> > > > >
>> > > > > public void validateCreateTopic(requestMetadata, ...) {
>> > > > > commonPolicy(requestMetadata.requestedState());
>> > > > > }
>> > > > >
>> > > > > public void validateAlterTopic(requestMetadata, ...) {
>> > > > > commonPolicy(requestMetadata.requestedState());
>> > > > > }
>> > > > >
>> > > > > private void commonPolicy(RequestedTopicState requestedState) {
>> > > > > // ...
>> > > > > }
>> > > > >
>> > > > > I think that's an important feature of the API because (I think)
>> very
>> > > > > often the policy maker is interested in defining the universe of
>> > > > prohibited
>> > > > > configurations without really caring about whether the request is
>> a
>> > > > create
>> > > > > or an alter. Having a single RequestedTopicState for both create
>> and
>> > > > > alter means they can do that trivially in one place. Having
>> different
>> > > > > methods in the two Request classes prevents this and forces the
>> > policy
>> > > > > maker to pick apart the different requestState objects before
>> calling
>> > > any
>> > > > > common method(s).
>> > > > >
>> > > > > I think my intention at the time (and it's many months ago now,
>> so I
>> > > > might
>> > > > > not have remembered fully) was that RequestedTopicState would
>> > basically
>> > > > > represent what the topic would look like after the requested
>> changes
>> > > were
>> > > > > applied (I accept this isn't how it's Javadoc'd in the KIP),
>> rather
>> > > than
>> > > > > representing the request itself. Thus if the request changed the
>> > > > assignment
>> > > > > of some of the partitions and the policy maker was interested in
>> > > > precisely
>> > > > > which partitions would be changed, and how, they would indeed
>> have to
>> > > > > compute that for themselves by looking up the current topic state
>> > from
>> > > > the
>> > > > > cluster state and seeing how they differed. Indeed they'd have to
>> do
>> > > this
>> > > > > diff even to figure out that the user was requesting a change to
>> the
>> > > > topic
>> > > > > assigned (or similarly for topic config, etc). To me this is
>> > acceptable
>> > > > > because I think most people writing such policies are just
>> interested
>> > > in
>> > > > > defining what is not allowed, so giving them a representation of
>> the
>> > > > > proposed topic state which they can readily check against is the
>> most
>> > > > > direct API. In this interpretation generatedReplicaAssignment()
>> would
>> > > > > just be some extra metadata annotating whether any difference
>> between
>> > > the
>> > > > > current and proposed states was directly from the user, or
>> generated
>> > on
>> > > > the
>> > > > > broker. You're right that it's ambiguous when the request didn't
>> > > actually
>> > > > > change the assignment but I didn't envisage policy makers using it
>> > > except
>> > > > > when the assignments differed anyway. To me it would be
>> acceptable to
>> > > > > Javadoc this.
>> > > > >
>> > > > > Given this interpretation of RequestedTopicState as "what the
>> topic
>> > > would
>> > > > > look like after the requested changes were applied" can you see
>> any
>> > > other
>> > > > > problems with the proposal? Or do you have use cases where the
>> policy
>> > > > maker
>> > > > > is more interested in what the request is changing?
>> > > > >
>> > > > > Kind regards,
>> > > > >
>> > > > > Tom
>> > > > >
>> > > > > On Fri, 7 Dec 2018 at 08:41, Tom Bentley <t....@gmail.com>
>> > > wrote:
>> > > > >
>> > > > >> Hi Anna and Mickael,
>> > > > >>
>> > > > >> Sorry for remaining silent on this for so long. I should have
>> time
>> > to
>> > > > >> look at this again next week.
>> > > > >>
>> > > > >> Kind regards,
>> > > > >>
>> > > > >> Tom
>> > > > >>
>> > > > >> On Mon, 3 Dec 2018 at 10:11, Mickael Maison <
>> > mickael.maison@gmail.com
>> > > >
>> > > > >> wrote:
>> > > > >>
>> > > > >>> Hi Tom,
>> > > > >>>
>> > > > >>> This is a very interesting KIP. If you are not going to continue
>> > > > >>> working on it, would it be ok for us to grab it and complete it?
>> > > > >>> Thanks
>> > > > >>> On Thu, Jun 14, 2018 at 7:06 PM Anna Povzner <anna@confluent.io
>> >
>> > > > wrote:
>> > > > >>> >
>> > > > >>> > Hi Tom,
>> > > > >>> >
>> > > > >>> > Just wanted to check what you think about the comments I made
>> in
>> > my
>> > > > >>> last
>> > > > >>> > message. I think this KIP is a big improvement to our current
>> > > policy
>> > > > >>> > interfaces, and really hope we can get this KIP in.
>> > > > >>> >
>> > > > >>> > Thanks,
>> > > > >>> > Anna
>> > > > >>> >
>> > > > >>> > On Thu, May 31, 2018 at 3:29 PM, Anna Povzner <
>> anna@confluent.io
>> > >
>> > > > >>> wrote:
>> > > > >>> >
>> > > > >>> > > Hi Tom,
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > Thanks for the KIP. I am aware that the voting thread was
>> > > started,
>> > > > >>> but
>> > > > >>> > > wanted to discuss couple of concerns here first.
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > I think the coupling of
>> > > > >>> RequestedTopicState#generatedReplicaAssignment()
>> > > > >>> > > and TopicState#replicasAssignments() does not work well in
>> case
>> > > > >>> where the
>> > > > >>> > > request deals only with a subset of partitions (e.g., add
>> > > > >>> partitions) or no
>> > > > >>> > > assignment at all (alter topic config). In particular:
>> > > > >>> > >
>> > > > >>> > > 1) Alter topic config use case: There is no replica
>> assignment
>> > in
>> > > > the
>> > > > >>> > > request, and generatedReplicaAssignment() returning either
>> > true
>> > > or
>> > > > >>> false
>> > > > >>> > > is both misleading. The user can interpret this as
>> assignment
>> > > being
>> > > > >>> > > generated or provided by the user originally (e.g., on topic
>> > > > >>> create), while
>> > > > >>> > > I don’t think we track such thing.
>> > > > >>> > >
>> > > > >>> > > 2) On add partitions, we may have manual assignment for new
>> > > > >>> partitions.
>> > > > >>> > > What I understood from the KIP,
>> generatedReplicaAssignment()
>> > > will
>> > > > >>> return
>> > > > >>> > > true or false based on whether new partitions were manually
>> > > > assigned
>> > > > >>> or
>> > > > >>> > > not, while TopicState#replicasAssignments() will return
>> replica
>> > > > >>> > > assignments for all partitions. I think it is confusing in a
>> > way
>> > > > that
>> > > > >>> > > assignment of old partitions could be auto-generated but new
>> > > > >>> partitions are
>> > > > >>> > > manually assigned.
>> > > > >>> > >
>> > > > >>> > > 3) Generalizing #2, suppose in a future, a user can
>> re-assign
>> > > > >>> replicas for
>> > > > >>> > > a set of partitions.
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > One way to address this with minimal changes to proposed
>> API is
>> > > to
>> > > > >>> rename
>> > > > >>> > > RequestedTopicState#generatedReplicaAssignment() to
>> > > > >>> RequestedTopicState#manualReplicaAssignment()
>> > > > >>> > > and change the API behavior and description to : “True if
>> the
>> > > > client
>> > > > >>> > > explicitly provided replica assignments in this request,
>> which
>> > > > means
>> > > > >>> that
>> > > > >>> > > some or all assignments returned by
>> > > > TopicState#replicasAssignments()
>> > > > >>> are
>> > > > >>> > > explicitly requested by the user”. The user then will have
>> to
>> > > diff
>> > > > >>> > > TopicState#replicasAssignments() from clusterState and
>> > > TopicState#
>> > > > >>> > > replicasAssignments() from RequestedTopicState, and assume
>> > that
>> > > > >>> > > assignments that are different are manually assigned (if
>> > > > >>> > > RequestedTopicState#manualReplicaAssignment() returns
>> true).
>> > We
>> > > > will
>> > > > >>> > > need to clearly document this and it still seems awkward.
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > I think a cleaner way is to make RequestedTopicState to
>> provide
>> > > > >>> replica
>> > > > >>> > > assignments only for partitions that were manually assigned
>> > > > replicas
>> > > > >>> in the
>> > > > >>> > > request that is being validated. Similarly, for alter topic
>> > > > >>> validation, it
>> > > > >>> > > would be nice to make it more clear for the user what has
>> been
>> > > > >>> changed. I
>> > > > >>> > > remember that you already raised that point earlier by
>> > comparing
>> > > > >>> current
>> > > > >>> > > proposed API with having separate methods for each specific
>> > > > command.
>> > > > >>> > > However, I agree that it will make it harder to change the
>> > > > interface
>> > > > >>> in the
>> > > > >>> > > future.
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > Could we explore the option of pushing methods that are
>> > currently
>> > > > in
>> > > > >>> > > TopicState to CreateTopicRequest and AlterTopicRequest?
>> > > TopicState
>> > > > >>> will
>> > > > >>> > > still be used for requesting current topic state via
>> > > ClusterState.
>> > > > >>> > >
>> > > > >>> > > Something like:
>> > > > >>> > >
>> > > > >>> > > interface CreateTopicRequest extends
>> AbstractRequestMetadata {
>> > > > >>> > >
>> > > > >>> > > // requested number of partitions or if manual assignment
>> is
>> > > > given,
>> > > > >>> > > number of partitions in the assignment
>> > > > >>> > >
>> > > > >>> > > int numPartitions();
>> > > > >>> > >
>> > > > >>> > > // requested replication factor, or if manual assignment
>> is
>> > > > given,
>> > > > >>> > > number of replicas in assignment for partition 0
>> > > > >>> > >
>> > > > >>> > > short replicationFactor();
>> > > > >>> > >
>> > > > >>> > > // replica assignment requested by the client, or null if
>> > > > >>> assignment is
>> > > > >>> > > auto-generated
>> > > > >>> > >
>> > > > >>> > > map<Integer, List<Integer>> manualReplicaAssignment();
>> > > > >>> > >
>> > > > >>> > > map<String, String> configs();
>> > > > >>> > >
>> > > > >>> > > }
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > interface AlterTopicRequest extends AbstractRequestMetadata
>> {
>> > > > >>> > >
>> > > > >>> > > // updated topic configs, or null if not changed
>> > > > >>> > >
>> > > > >>> > > map<String, String> updatedConfigs();
>> > > > >>> > >
>> > > > >>> > > // proposed replica assignment in this request, or null.
>> For
>> > > > >>> adding new
>> > > > >>> > > partitions request, this is proposed replica assignment for
>> new
>> > > > >>> partitions.
>> > > > >>> > > For replica re-assignment case, this is proposed new
>> > assignment.
>> > > > >>> > >
>> > > > >>> > > map<Integer, List<Integer>> proposedReplicaAssignment();
>> > > > >>> > >
>> > > > >>> > > // new number of partitions (due to increase/decrease), or
>> > null
>> > > > if
>> > > > >>> > > number of partitions not changed
>> > > > >>> > >
>> > > > >>> > > Integer updatedNumPartitions()
>> > > > >>> > >
>> > > > >>> > > }
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > I did not spend much time on my AlterTopicRequest interface
>> > > > >>> proposal, but
>> > > > >>> > > the idea is basically to return only the parts which were
>> > > changed.
>> > > > >>> The
>> > > > >>> > > advantage of this approach over having separate methods for
>> > each
>> > > > >>> specific
>> > > > >>> > > alter topic request is that it is more flexible for future
>> > mixing
>> > > > of
>> > > > >>> what
>> > > > >>> > > can be updated in the topic state.
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > What do you think?
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > Thanks,
>> > > > >>> > >
>> > > > >>> > > Anna
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>> > > On Mon, Oct 9, 2017 at 1:39 AM, Tom Bentley <
>> > > t.j.bentley@gmail.com
>> > > > >
>> > > > >>> wrote:
>> > > > >>> > >
>> > > > >>> > >> I've added RequestedTopicState, as discussed in my last
>> email.
>> > > > >>> > >>
>> > > > >>> > >> I've also added a paragraph to the migration plan about old
>> > > > clients
>> > > > >>> making
>> > > > >>> > >> policy-violating delete topics or delete records request.
>> > > > >>> > >>
>> > > > >>> > >> If no further comments a forthcoming in the next day or two
>> > > then I
>> > > > >>> will
>> > > > >>> > >> start a vote.
>> > > > >>> > >>
>> > > > >>> > >> Thanks,
>> > > > >>> > >>
>> > > > >>> > >> Tom
>> > > > >>> > >>
>> > > > >>> > >> On 5 October 2017 at 12:41, Tom Bentley <
>> > t.j.bentley@gmail.com>
>> > > > >>> wrote:
>> > > > >>> > >>
>> > > > >>> > >> > I'd like to raise a somewhat subtle point about how the
>> > > proposed
>> > > > >>> API
>> > > > >>> > >> > should behave.
>> > > > >>> > >> >
>> > > > >>> > >> > The current CreateTopicPolicy gets passed either the
>> request
>> > > > >>> partition
>> > > > >>> > >> > count and replication factor, or the requested
>> assignment.
>> > So
>> > > if
>> > > > >>> the
>> > > > >>> > >> > request had specified partition count and replication
>> > factor,
>> > > > the
>> > > > >>> policy
>> > > > >>> > >> > sees a null replicaAssignments(). Likewise if the request
>> > > > >>> specified a
>> > > > >>> > >> > replica assignment the policy would get back null from
>> > > > >>> numPartitions()
>> > > > >>> > >> and
>> > > > >>> > >> > replicationFactor().
>> > > > >>> > >> >
>> > > > >>> > >> > These semantics mean the policy can't reject an
>> assignment
>> > > that
>> > > > >>> happened
>> > > > >>> > >> > to be auto-generated (or rather, it's obvious to the
>> policy
>> > > that
>> > > > >>> the
>> > > > >>> > >> > assignment is auto generated, because it can't see such
>> > > > >>> assignments),
>> > > > >>> > >> > though it can reject a request because the assignment was
>> > > > >>> > >> auto-generated,
>> > > > >>> > >> > or vice versa.
>> > > > >>> > >> >
>> > > > >>> > >> > Retaining these semantics makes the TopicState less
>> > symmetric
>> > > > >>> between
>> > > > >>> > >> it's
>> > > > >>> > >> > use in requestedState() and the current state available
>> from
>> > > the
>> > > > >>> > >> > ClusterState, and also less symmetric between its use for
>> > > > >>> createTopic()
>> > > > >>> > >> and
>> > > > >>> > >> > for alterTopic(). This can make it harder to write a
>> policy.
>> > > For
>> > > > >>> > >> example,
>> > > > >>> > >> > if I want the policy "the number of partitions must be <
>> > 100",
>> > > > if
>> > > > >>> the
>> > > > >>> > >> > requestedState().numPartitions() can be null I need to
>> cope
>> > > with
>> > > > >>> that
>> > > > >>> > >> > and figure it out from inspecting the
>> > replicasAssignments().
>> > > It
>> > > > >>> would
>> > > > >>> > >> be
>> > > > >>> > >> > much better for the policy writer to just be able to
>> write:
>> > > > >>> > >> >
>> > > > >>> > >> > if (request.requestedState().numPartitions() >= 100)
>> > > > >>> > >> > throw new PolicyViolationException("#partitions
>> must
>> > > be
>> > > > <
>> > > > >>> 100")
>> > > > >>> > >> >
>> > > > >>> > >> > An alternative would be to keep the symmetry (and thus
>> > > > >>> > >> TopicState.replicasAssignments()
>> > > > >>> > >> > would never return null, and TopicState.numPartitions()
>> and
>> > > > >>> > >> > TopicState.replicationFactor() could each be primitives),
>> > but
>> > > > >>> expose the
>> > > > >>> > >> > auto-generatedness of the replicaAssignments()
>> explicitly,
>> > > > >>> perhaps by
>> > > > >>> > >> using
>> > > > >>> > >> > a subtype of TopicState for the return type of
>> > > requestedState():
>> > > > >>> > >> >
>> > > > >>> > >> > interface RequestedTopicState extends TopicState {
>> > > > >>> > >> > /**
>> > > > >>> > >> > * True if the {@link
>> > > TopicState#replicasAssignments()}
>> > > > >>> > >> > * in this request we generated by the broker,
>> false
>> > > if
>> > > > >>> > >> > * they were explicitly requested by the client.
>> > > > >>> > >> > */
>> > > > >>> > >> > boolean generatedReplicaAssignments();
>> > > > >>> > >> > }
>> > > > >>> > >> >
>> > > > >>> > >> > Thoughts?
>> > > > >>> > >> >
>> > > > >>> > >> > On 4 October 2017 at 11:06, Tom Bentley <
>> > > t.j.bentley@gmail.com>
>> > > > >>> wrote:
>> > > > >>> > >> >
>> > > > >>> > >> >> Good point. Then I guess I can do those items too. I
>> would
>> > > also
>> > > > >>> need to
>> > > > >>> > >> >> do the same changes for DeleteRecordsRequest and
>> Response.
>> > > > >>> > >> >>
>> > > > >>> > >> >> On 4 October 2017 at 10:37, Ismael Juma <
>> ismael@juma.me.uk
>> > >
>> > > > >>> wrote:
>> > > > >>> > >> >>
>> > > > >>> > >> >>> Those two points are related to policies in the
>> following
>> > > > sense:
>> > > > >>> > >> >>>
>> > > > >>> > >> >>> 1. A policy that can't send errors to clients is much
>> less
>> > > > >>> useful
>> > > > >>> > >> >>> 2. Testing policies is much easier with `validateOnly`
>> > > > >>> > >> >>>
>> > > > >>> > >> >>> Ismael
>> > > > >>> > >> >>>
>> > > > >>> > >> >>> On Wed, Oct 4, 2017 at 9:20 AM, Tom Bentley <
>> > > > >>> t.j.bentley@gmail.com>
>> > > > >>> > >> >>> wrote:
>> > > > >>> > >> >>>
>> > > > >>> > >> >>> > Thanks Edoardo,
>> > > > >>> > >> >>> >
>> > > > >>> > >> >>> > I've added that motivation to the KIP.
>> > > > >>> > >> >>> >
>> > > > >>> > >> >>> > KIP-201 doesn't address two points raised in KIP-170:
>> > > > Adding a
>> > > > >>> > >> >>> > validationOnly flag to
>> > > > >>> > >> >>> > DeleteTopicRequest and adding an error message to
>> > > > >>> > >> DeleteTopicResponse.
>> > > > >>> > >> >>> > Since those are not policy-related I think they're
>> best
>> > > left
>> > > > >>> out of
>> > > > >>> > >> >>> > KIP-201. I suppose it is up to you and Mickael
>> whether
>> > to
>> > > > >>> narrow the
>> > > > >>> > >> >>> scope
>> > > > >>> > >> >>> > of KIP-170 to address those points.
>> > > > >>> > >> >>> >
>> > > > >>> > >> >>> > Thanks again,
>> > > > >>> > >> >>> >
>> > > > >>> > >> >>> > Tom
>> > > > >>> > >> >>> >
>> > > > >>> > >> >>> > On 4 October 2017 at 08:20, Edoardo Comar <
>> > > > ECOMAR@uk.ibm.com>
>> > > > >>> > >> wrote:
>> > > > >>> > >> >>> >
>> > > > >>> > >> >>> > > Thanks Tom,
>> > > > >>> > >> >>> > > looks got to me and KIP-201 could supersede KIP-170
>> > > > >>> > >> >>> > > but could you please add a missing motivation
>> bullet
>> > > that
>> > > > >>> was
>> > > > >>> > >> behind
>> > > > >>> > >> >>> > > KIP-170:
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > introducing ClusterState to allow validation of
>> > > > >>> create/alter topic
>> > > > >>> > >> >>> > request
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > not just against the request metadata but also
>> > > > >>> > >> >>> > > against the current amount of resources already
>> used
>> > in
>> > > > the
>> > > > >>> > >> cluster
>> > > > >>> > >> >>> (eg
>> > > > >>> > >> >>> > > number of partitions).
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > thanks
>> > > > >>> > >> >>> > > Edo
>> > > > >>> > >> >>> > > --------------------------------------------------
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > Edoardo Comar
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > IBM Message Hub
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > IBM UK Ltd, Hursley Park, SO21 2JN
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > From: Tom Bentley <t....@gmail.com>
>> > > > >>> > >> >>> > > To: dev@kafka.apache.org
>> > > > >>> > >> >>> > > Date: 02/10/2017 15:15
>> > > > >>> > >> >>> > > Subject: Re: [DISCUSS] KIP-201:
>> Rationalising
>> > > > Policy
>> > > > >>> > >> >>> interfaces
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > Hi All,
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > I've updated KIP-201 again so there is now a single
>> > > policy
>> > > > >>> > >> interface
>> > > > >>> > >> >>> (and
>> > > > >>> > >> >>> > > thus a single key by which to configure it) for
>> topic
>> > > > >>> creation,
>> > > > >>> > >> >>> > > modification, deletion and record deletion, which
>> each
>> > > > have
>> > > > >>> their
>> > > > >>> > >> own
>> > > > >>> > >> >>> > > validation method.
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > There are still a few loose ends:
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > 1. I currently propose validateAlterTopic(), but it
>> > > would
>> > > > be
>> > > > >>> > >> >>> possible to
>> > > > >>> > >> >>> > > be
>> > > > >>> > >> >>> > > more fine grained about this:
>> validateAlterConfig(),
>> > > > >>> > >> >>> validAddPartitions()
>> > > > >>> > >> >>> > > and validateReassignPartitions(), for example.
>> > Obviously
>> > > > >>> this
>> > > > >>> > >> >>> results in
>> > > > >>> > >> >>> > a
>> > > > >>> > >> >>> > > policy method per operation, and makes it more
>> clear
>> > > what
>> > > > >>> is being
>> > > > >>> > >> >>> > > changed.
>> > > > >>> > >> >>> > > I guess the down side is its more work for
>> > implementer,
>> > > > and
>> > > > >>> > >> >>> potentially
>> > > > >>> > >> >>> > > makes it harder to change the interface in the
>> future.
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > 2. A couple of TODOs about what the TopicState
>> > interface
>> > > > >>> should
>> > > > >>> > >> >>> return
>> > > > >>> > >> >>> > > when
>> > > > >>> > >> >>> > > a topic's partitions are being reassigned.
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > Your thoughts on these or any other points are
>> > welcome.
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > Thanks,
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > Tom
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > On 27 September 2017 at 11:45, Paolo Patierno <
>> > > > >>> ppatierno@live.com
>> > > > >>> > >> >
>> > > > >>> > >> >>> > wrote:
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > > Hi Ismael,
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > 1. I don't have a real requirement now but
>> > > "deleting"
>> > > > >>> is an
>> > > > >>> > >> >>> > operation
>> > > > >>> > >> >>> > > > that could be really dangerous so it's always
>> better
>> > > > >>> having a
>> > > > >>> > >> way
>> > > > >>> > >> >>> for
>> > > > >>> > >> >>> > > > having more control on that. I know that we have
>> the
>> > > > >>> authorizer
>> > > > >>> > >> >>> used
>> > > > >>> > >> >>> > for
>> > > > >>> > >> >>> > > > that (delete on topic) but fine grained control
>> > could
>> > > be
>> > > > >>> better
>> > > > >>> > >> >>> (even
>> > > > >>> > >> >>> > > > already happens for topic deletion).
>> > > > >>> > >> >>> > > > 2. I know about the problem of restarting
>> broker
>> > > due
>> > > > to
>> > > > >>> > >> changes
>> > > > >>> > >> >>> on
>> > > > >>> > >> >>> > > > policies but what do you mean by doing that on
>> the
>> > > > >>> clients ?
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > Paolo Patierno
>> > > > >>> > >> >>> > > > Senior Software Engineer (IoT) @ Red Hat
>> > > > >>> > >> >>> > > > Microsoft MVP on Azure & IoT
>> > > > >>> > >> >>> > > > Microsoft Azure Advisor
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > Twitter : @ppatierno<
>> > > > >>> > >> >>> > >
>> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter
>> > > > >>> .
>> > > > >>> > >> >>> > > com_ppatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
>> > > > >>> > >> >>> > >
>> > > > >>>
>> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
>> > > > >>> > >> >>> > >
>> yh7dKgV77XtsUnJ9Rab1gheY&s=43hzTLEDKw2v5Vh0zwkMTaaKD-
>> > > > >>> > >> >>> > HdJD8d_F4-Bsw25-Y&e=
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > Linkedin : paolopatierno<
>> > > > >>> > >> >>> > >
>> > https://urldefense.proofpoint.com/v2/url?u=http-3A__it.
>> > > > >>> > >> >>> > >
>> > > > linkedin.com_in_paolopatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1
>> > > > >>> > >> ZOg&r=
>> > > > >>> > >> >>> > >
>> > > > >>>
>> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
>> > > > >>> > >> >>> > >
>> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Ig0N7Nwf9EHfTJ2pH3jRM1JIdlzXw6
>> > > > >>> > >> >>> > R5Drocu0TMRLk&e=
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > Blog : DevExperience<
>> > > > >>> > >> >>> > >
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__
>> > > > >>> > >> >>> > >
>> > > > >>>
>> paolopatierno.wordpress.com_&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
>> > > > >>> > >> >>> > >
>> > > > >>>
>> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
>> > > > >>> > >> >>> > >
>> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Tc9NrTtG2GP7-zRjOHkXHfYI0rncO8_
>> > > > >>> > >> >>> > jKpedna692z4&e=
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > ________________________________
>> > > > >>> > >> >>> > > > From: ismaelj@gmail.com <is...@gmail.com> on
>> > behalf
>> > > > of
>> > > > >>> Ismael
>> > > > >>> > >> >>> Juma <
>> > > > >>> > >> >>> > > > ismael@juma.me.uk>
>> > > > >>> > >> >>> > > > Sent: Wednesday, September 27, 2017 10:30 AM
>> > > > >>> > >> >>> > > > To: dev@kafka.apache.org
>> > > > >>> > >> >>> > > > Subject: Re: [DISCUSS] KIP-201: Rationalising
>> Policy
>> > > > >>> interfaces
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > A couple of questions:
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > 1. Is this a concrete requirement from a user or
>> is
>> > it
>> > > > >>> > >> >>> hypothetical?
>> > > > >>> > >> >>> > > > 2. You sure you would want to do this in the
>> broker
>> > > > >>> instead of
>> > > > >>> > >> the
>> > > > >>> > >> >>> > > clients?
>> > > > >>> > >> >>> > > > It's worth remembering that updating broker
>> policies
>> > > > >>> involves a
>> > > > >>> > >> >>> rolling
>> > > > >>> > >> >>> > > > restart of the cluster, so it's not the right
>> place
>> > > for
>> > > > >>> things
>> > > > >>> > >> that
>> > > > >>> > >> >>> > > change
>> > > > >>> > >> >>> > > > frequently.
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > Ismael
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > On Wed, Sep 27, 2017 at 11:26 AM, Paolo Patierno
>> <
>> > > > >>> > >> >>> ppatierno@live.com>
>> > > > >>> > >> >>> > > > wrote:
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > > Hi Ismael,
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > > > regarding motivations for delete records, as I
>> > said
>> > > > >>> during the
>> > > > >>> > >> >>> > > discussion
>> > > > >>> > >> >>> > > > > on KIP-204, it gives the possibility to avoid
>> > > deleting
>> > > > >>> > >> messages
>> > > > >>> > >> >>> for
>> > > > >>> > >> >>> > > > > specific partitions (inside the topic) and
>> > starting
>> > > > >>> from a
>> > > > >>> > >> >>> specific
>> > > > >>> > >> >>> > > > offset.
>> > > > >>> > >> >>> > > > > I could think on some users solutions where
>> they
>> > > know
>> > > > >>> exactly
>> > > > >>> > >> >>> what
>> > > > >>> > >> >>> > the
>> > > > >>> > >> >>> > > > > partitions means in a specific topic (because
>> they
>> > > are
>> > > > >>> using a
>> > > > >>> > >> >>> custom
>> > > > >>> > >> >>> > > > > partitioner on the producer side) so they know
>> > what
>> > > > >>> kind of
>> > > > >>> > >> >>> messages
>> > > > >>> > >> >>> > > are
>> > > > >>> > >> >>> > > > > inside a partition allowing to delete them but
>> not
>> > > the
>> > > > >>> others.
>> > > > >>> > >> >>> In
>> > > > >>> > >> >>> > > such a
>> > > > >>> > >> >>> > > > > policy a user could also check the timestamp
>> > related
>> > > > to
>> > > > >>> the
>> > > > >>> > >> >>> offset
>> > > > >>> > >> >>> > for
>> > > > >>> > >> >>> > > > > allowing or not deletion on time base.
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > > > Paolo Patierno
>> > > > >>> > >> >>> > > > > Senior Software Engineer (IoT) @ Red Hat
>> > > > >>> > >> >>> > > > > Microsoft MVP on Azure & IoT
>> > > > >>> > >> >>> > > > > Microsoft Azure Advisor
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > > > Twitter : @ppatierno<
>> > > > >>> > >> >>> > >
>> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter
>> > > > >>> .
>> > > > >>> > >> >>> > > com_ppatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
>> > > > >>> > >> >>> > >
>> > > > >>>
>> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
>> > > > >>> > >> >>> > >
>> yh7dKgV77XtsUnJ9Rab1gheY&s=43hzTLEDKw2v5Vh0zwkMTaaKD-
>> > > > >>> > >> >>> > HdJD8d_F4-Bsw25-Y&e=
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > > Linkedin : paolopatierno<
>> > > > >>> > >> >>> > >
>> > https://urldefense.proofpoint.com/v2/url?u=http-3A__it.
>> > > > >>> > >> >>> > >
>> > > > linkedin.com_in_paolopatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1
>> > > > >>> > >> ZOg&r=
>> > > > >>> > >> >>> > >
>> > > > >>>
>> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
>> > > > >>> > >> >>> > >
>> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Ig0N7Nwf9EHfTJ2pH3jRM1JIdlzXw6
>> > > > >>> > >> >>> > R5Drocu0TMRLk&e=
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > > Blog : DevExperience<
>> > > > >>> > >> >>> > >
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__
>> > > > >>> > >> >>> > >
>> > > > >>>
>> paolopatierno.wordpress.com_&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
>> > > > >>> > >> >>> > >
>> > > > >>>
>> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
>> > > > >>> > >> >>> > >
>> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Tc9NrTtG2GP7-zRjOHkXHfYI0rncO8_
>> > > > >>> > >> >>> > jKpedna692z4&e=
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > > > ________________________________
>> > > > >>> > >> >>> > > > > From: ismaelj@gmail.com <is...@gmail.com> on
>> > > behalf
>> > > > >>> of
>> > > > >>> > >> Ismael
>> > > > >>> > >> >>> > Juma <
>> > > > >>> > >> >>> > > > > ismael@juma.me.uk>
>> > > > >>> > >> >>> > > > > Sent: Wednesday, September 27, 2017 10:18 AM
>> > > > >>> > >> >>> > > > > To: dev@kafka.apache.org
>> > > > >>> > >> >>> > > > > Subject: Re: [DISCUSS] KIP-201: Rationalising
>> > Policy
>> > > > >>> > >> interfaces
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > > > 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 <
>> > > > >>> > >> ismael@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....@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 <
>> > > > >>> > >> >>> ECOMAR@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.bentley@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
>> > > > >>> > >> >>> > > > > >> >
>> > > > >>> > >> >>> > > > > >>
>> > > > >>> > >> >>> > > > > >
>> > > > >>> > >> >>> > > > > >
>> > > > >>> > >> >>> > > > >
>> > > > >>> > >> >>> > > >
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> > > 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
>> > > > >>> > >> >>> > >
>> > > > >>> > >> >>> >
>> > > > >>> > >> >>>
>> > > > >>> > >> >>
>> > > > >>> > >> >>
>> > > > >>> > >> >
>> > > > >>> > >>
>> > > > >>> > >
>> > > > >>> > >
>> > > > >>>
>> > > > >>
>> > > >
>> > >
>> >
>>
>
Re: [DISCUSS] KIP-201: Rationalising Policy interfaces
Posted by Tom Bentley <tb...@redhat.com>.
Hi Mickael,
Sure, that makes sense so I've updated the KIP.
Kind regards,
Tom
On Mon, Aug 12, 2019 at 12:23 PM Mickael Maison <mi...@gmail.com>
wrote:
> Hi Tom,
>
> Thanks for following up on this KIP. This is a great improvement that
> will make policies more powerful and at the same time easier to
> manage.
>
> I just have one question:
> In AbstractRequestMetadata.principal() javadoc, it says the principal
> will be "null" for non authenticated session. Can't we just have the
> default Principal for the Session instead of null? It's possible to
> have Principals for PLAINTEXT sessions or use the default ANONYMOUS
> Principal.
>
> Thanks
>
>
> On Mon, Aug 12, 2019 at 10:52 AM Tom Bentley <tb...@redhat.com> wrote:
> >
> > Hi folks,
> >
> > As far as I can see the motivation for KIP-201 is still valid, and as far
> > as I'm aware the changes I made to the KIP back in April addressed the
> > previous comments. Since the issue still needs to be addressed I intend
> to
> > start another vote thread in the near future, but before I do I thought
> I'd
> > check whether anyone has any more comments. So please let me know any
> > feedback for this KIP.
> >
> > Many thanks,
> >
> > Tom
> >
> > On Fri, Apr 12, 2019 at 10:46 AM Tom Bentley <tb...@redhat.com>
> wrote:
> >
> > > Hi Rajini,
> > >
> > > I've made a number of changes to the KIP.
> > >
> > > 1. I've added RequestedTopicState.requestedConfigs(). This is obviously
> > > unrelated to supporting alter broker, but I think it goes some way to
> > > addressing one of the points Anna made last year.
> > > Anna, wdyt?
> > >
> > > 2. I've added BrokerState, RequestedBrokerState and
> > > BrokerManagementPolicy. These are largely similar to the interfaces for
> > > topic management, but the lifecycle of a BrokerManagementPolicy needs
> to be
> > > different.
> > >
> > > That's because a BrokerManagementPolicy ought to be Configurable with
> the
> > > broker config, but obviously the broker config can change. Because a
> > > cluster-scoped config might be changed via a different broker we need
> to
> > > hook into the Zookeeper change notification on the broker configs to
> > > instantiate a new BrokerManagementPolicy when broker policy changes.
> We'd
> > > need to cope with policy implementation change happening concurrently
> with
> > > policy enforcement.
> > > And technically there's a race here: Sending changes to cluster-scoped
> > > configs to multiple brokers could result in non-deterministic policy
> > > enforcement.
> > >
> > > One way to avoid that would be to require changes to cluster-scoped
> > > configs to be sent to the controller.
> > > This complexity is annoying because it seems likely that many policy
> > > implementations won't _actually_ depend on the broker config.
> > >
> > > Thoughts?
> > >
> > > Kind regards,
> > >
> > > Tom
> > >
> > > On Wed, Apr 10, 2019 at 9:48 AM Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > >> Thanks Tom.
> > >>
> > >> Once you have updated the KIP to support broker config updates, it
> may be
> > >> good to start a new vote thread since the other one is quite old and
> > >> perhaps the KIP has changed since then.
> > >>
> > >>
> > >> On Wed, Apr 10, 2019 at 3:58 AM Tom Bentley <tb...@redhat.com>
> wrote:
> > >>
> > >> > Hi Rajini,
> > >> >
> > >> > I'd be happy to do that. I'll try to get it done in the next few
> days.
> > >> >
> > >> > Although there's been quite a lot of interest this, the vote thread
> > >> never
> > >> > got any binding +1, so it's been stuck in limbo for a long time. It
> > >> would
> > >> > be great to get this moving again.
> > >> >
> > >> > Kind regards,
> > >> >
> > >> > Tom
> > >> >
> > >> > On Tue, Apr 9, 2019 at 3:04 PM Rajini Sivaram <
> rajinisivaram@gmail.com>
> > >> > wrote:
> > >> >
> > >> > > Hi Tom,
> > >> > >
> > >> > > Are you planning to extend this KIP to also include dynamic broker
> > >> config
> > >> > > update (currently covered under AlterConfigPolicy)?
> > >> > >
> > >> > > May be worth sending another note to make progress on this KIP
> since
> > >> it
> > >> > has
> > >> > > been around a while and reading through the threads, it looks like
> > >> there
> > >> > > has been a lot of interest in it.
> > >> > >
> > >> > > Thank you,
> > >> > >
> > >> > > Rajini
> > >> > >
> > >> > >
> > >> > > On Wed, Jan 9, 2019 at 11:25 AM Tom Bentley <
> t.j.bentley@gmail.com>
> > >> > wrote:
> > >> > >
> > >> > > > Hi Anna and Mickael,
> > >> > > >
> > >> > > > Anna, did you have any comments about the points I made?
> > >> > > >
> > >> > > > Mickael, we really need the vote to be passed before there's
> even
> > >> any
> > >> > > work
> > >> > > > to do. With the exception of Ismael, the KIP didn't seem to get
> the
> > >> > > > attention of any of the other committers.
> > >> > > >
> > >> > > > Kind regards,
> > >> > > >
> > >> > > > Tom
> > >> > > >
> > >> > > > On Thu, 13 Dec 2018 at 18:11, Tom Bentley <
> t.j.bentley@gmail.com>
> > >> > wrote:
> > >> > > >
> > >> > > > > Hi Anna,
> > >> > > > >
> > >> > > > > Firstly, let me apologise again about having missed your
> previous
> > >> > > emails
> > >> > > > > about this.
> > >> > > > >
> > >> > > > > Thank you for the feedback. You raise some valid points about
> > >> > > ambiguity.
> > >> > > > > The problem with pulling the metadata into CreateTopicRequest
> and
> > >> > > > > AlterTopicRequest is that you lose the benefit of being able
> to
> > >> eaily
> > >> > > > write
> > >> > > > > a common policy across creation and alter cases. For example,
> with
> > >> > the
> > >> > > > > proposed design the policy maker could write code like this
> > >> (forgive
> > >> > my
> > >> > > > > pseudo-Java)
> > >> > > > >
> > >> > > > > public void validateCreateTopic(requestMetadata, ...) {
> > >> > > > > commonPolicy(requestMetadata.requestedState());
> > >> > > > > }
> > >> > > > >
> > >> > > > > public void validateAlterTopic(requestMetadata, ...) {
> > >> > > > > commonPolicy(requestMetadata.requestedState());
> > >> > > > > }
> > >> > > > >
> > >> > > > > private void commonPolicy(RequestedTopicState
> requestedState) {
> > >> > > > > // ...
> > >> > > > > }
> > >> > > > >
> > >> > > > > I think that's an important feature of the API because (I
> think)
> > >> very
> > >> > > > > often the policy maker is interested in defining the universe
> of
> > >> > > > prohibited
> > >> > > > > configurations without really caring about whether the
> request is
> > >> a
> > >> > > > create
> > >> > > > > or an alter. Having a single RequestedTopicState for both
> create
> > >> and
> > >> > > > > alter means they can do that trivially in one place. Having
> > >> different
> > >> > > > > methods in the two Request classes prevents this and forces
> the
> > >> > policy
> > >> > > > > maker to pick apart the different requestState objects before
> > >> calling
> > >> > > any
> > >> > > > > common method(s).
> > >> > > > >
> > >> > > > > I think my intention at the time (and it's many months ago
> now,
> > >> so I
> > >> > > > might
> > >> > > > > not have remembered fully) was that RequestedTopicState would
> > >> > basically
> > >> > > > > represent what the topic would look like after the requested
> > >> changes
> > >> > > were
> > >> > > > > applied (I accept this isn't how it's Javadoc'd in the KIP),
> > >> rather
> > >> > > than
> > >> > > > > representing the request itself. Thus if the request changed
> the
> > >> > > > assignment
> > >> > > > > of some of the partitions and the policy maker was interested
> in
> > >> > > > precisely
> > >> > > > > which partitions would be changed, and how, they would indeed
> > >> have to
> > >> > > > > compute that for themselves by looking up the current topic
> state
> > >> > from
> > >> > > > the
> > >> > > > > cluster state and seeing how they differed. Indeed they'd
> have to
> > >> do
> > >> > > this
> > >> > > > > diff even to figure out that the user was requesting a change
> to
> > >> the
> > >> > > > topic
> > >> > > > > assigned (or similarly for topic config, etc). To me this is
> > >> > acceptable
> > >> > > > > because I think most people writing such policies are just
> > >> interested
> > >> > > in
> > >> > > > > defining what is not allowed, so giving them a representation
> of
> > >> the
> > >> > > > > proposed topic state which they can readily check against is
> the
> > >> most
> > >> > > > > direct API. In this interpretation
> generatedReplicaAssignment()
> > >> would
> > >> > > > > just be some extra metadata annotating whether any difference
> > >> between
> > >> > > the
> > >> > > > > current and proposed states was directly from the user, or
> > >> generated
> > >> > on
> > >> > > > the
> > >> > > > > broker. You're right that it's ambiguous when the request
> didn't
> > >> > > actually
> > >> > > > > change the assignment but I didn't envisage policy makers
> using it
> > >> > > except
> > >> > > > > when the assignments differed anyway. To me it would be
> > >> acceptable to
> > >> > > > > Javadoc this.
> > >> > > > >
> > >> > > > > Given this interpretation of RequestedTopicState as "what the
> > >> topic
> > >> > > would
> > >> > > > > look like after the requested changes were applied" can you
> see
> > >> any
> > >> > > other
> > >> > > > > problems with the proposal? Or do you have use cases where the
> > >> policy
> > >> > > > maker
> > >> > > > > is more interested in what the request is changing?
> > >> > > > >
> > >> > > > > Kind regards,
> > >> > > > >
> > >> > > > > Tom
> > >> > > > >
> > >> > > > > On Fri, 7 Dec 2018 at 08:41, Tom Bentley <
> t.j.bentley@gmail.com>
> > >> > > wrote:
> > >> > > > >
> > >> > > > >> Hi Anna and Mickael,
> > >> > > > >>
> > >> > > > >> Sorry for remaining silent on this for so long. I should have
> > >> time
> > >> > to
> > >> > > > >> look at this again next week.
> > >> > > > >>
> > >> > > > >> Kind regards,
> > >> > > > >>
> > >> > > > >> Tom
> > >> > > > >>
> > >> > > > >> On Mon, 3 Dec 2018 at 10:11, Mickael Maison <
> > >> > mickael.maison@gmail.com
> > >> > > >
> > >> > > > >> wrote:
> > >> > > > >>
> > >> > > > >>> Hi Tom,
> > >> > > > >>>
> > >> > > > >>> This is a very interesting KIP. If you are not going to
> continue
> > >> > > > >>> working on it, would it be ok for us to grab it and
> complete it?
> > >> > > > >>> Thanks
> > >> > > > >>> On Thu, Jun 14, 2018 at 7:06 PM Anna Povzner <
> anna@confluent.io
> > >> >
> > >> > > > wrote:
> > >> > > > >>> >
> > >> > > > >>> > Hi Tom,
> > >> > > > >>> >
> > >> > > > >>> > Just wanted to check what you think about the comments I
> made
> > >> in
> > >> > my
> > >> > > > >>> last
> > >> > > > >>> > message. I think this KIP is a big improvement to our
> current
> > >> > > policy
> > >> > > > >>> > interfaces, and really hope we can get this KIP in.
> > >> > > > >>> >
> > >> > > > >>> > Thanks,
> > >> > > > >>> > Anna
> > >> > > > >>> >
> > >> > > > >>> > On Thu, May 31, 2018 at 3:29 PM, Anna Povzner <
> > >> anna@confluent.io
> > >> > >
> > >> > > > >>> wrote:
> > >> > > > >>> >
> > >> > > > >>> > > Hi Tom,
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > Thanks for the KIP. I am aware that the voting thread
> was
> > >> > > started,
> > >> > > > >>> but
> > >> > > > >>> > > wanted to discuss couple of concerns here first.
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > I think the coupling of
> > >> > > > >>> RequestedTopicState#generatedReplicaAssignment()
> > >> > > > >>> > > and TopicState#replicasAssignments() does not work well
> in
> > >> case
> > >> > > > >>> where the
> > >> > > > >>> > > request deals only with a subset of partitions (e.g.,
> add
> > >> > > > >>> partitions) or no
> > >> > > > >>> > > assignment at all (alter topic config). In particular:
> > >> > > > >>> > >
> > >> > > > >>> > > 1) Alter topic config use case: There is no replica
> > >> assignment
> > >> > in
> > >> > > > the
> > >> > > > >>> > > request, and generatedReplicaAssignment() returning
> either
> > >> > true
> > >> > > or
> > >> > > > >>> false
> > >> > > > >>> > > is both misleading. The user can interpret this as
> > >> assignment
> > >> > > being
> > >> > > > >>> > > generated or provided by the user originally (e.g., on
> topic
> > >> > > > >>> create), while
> > >> > > > >>> > > I don’t think we track such thing.
> > >> > > > >>> > >
> > >> > > > >>> > > 2) On add partitions, we may have manual assignment for
> new
> > >> > > > >>> partitions.
> > >> > > > >>> > > What I understood from the KIP,
> > >> generatedReplicaAssignment()
> > >> > > will
> > >> > > > >>> return
> > >> > > > >>> > > true or false based on whether new partitions were
> manually
> > >> > > > assigned
> > >> > > > >>> or
> > >> > > > >>> > > not, while TopicState#replicasAssignments() will return
> > >> replica
> > >> > > > >>> > > assignments for all partitions. I think it is confusing
> in a
> > >> > way
> > >> > > > that
> > >> > > > >>> > > assignment of old partitions could be auto-generated
> but new
> > >> > > > >>> partitions are
> > >> > > > >>> > > manually assigned.
> > >> > > > >>> > >
> > >> > > > >>> > > 3) Generalizing #2, suppose in a future, a user can
> > >> re-assign
> > >> > > > >>> replicas for
> > >> > > > >>> > > a set of partitions.
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > One way to address this with minimal changes to proposed
> > >> API is
> > >> > > to
> > >> > > > >>> rename
> > >> > > > >>> > > RequestedTopicState#generatedReplicaAssignment() to
> > >> > > > >>> RequestedTopicState#manualReplicaAssignment()
> > >> > > > >>> > > and change the API behavior and description to : “True
> if
> > >> the
> > >> > > > client
> > >> > > > >>> > > explicitly provided replica assignments in this request,
> > >> which
> > >> > > > means
> > >> > > > >>> that
> > >> > > > >>> > > some or all assignments returned by
> > >> > > > TopicState#replicasAssignments()
> > >> > > > >>> are
> > >> > > > >>> > > explicitly requested by the user”. The user then will
> have
> > >> to
> > >> > > diff
> > >> > > > >>> > > TopicState#replicasAssignments() from clusterState and
> > >> > > TopicState#
> > >> > > > >>> > > replicasAssignments() from RequestedTopicState, and
> assume
> > >> > that
> > >> > > > >>> > > assignments that are different are manually assigned (if
> > >> > > > >>> > > RequestedTopicState#manualReplicaAssignment() returns
> > >> true).
> > >> > We
> > >> > > > will
> > >> > > > >>> > > need to clearly document this and it still seems
> awkward.
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > I think a cleaner way is to make RequestedTopicState to
> > >> provide
> > >> > > > >>> replica
> > >> > > > >>> > > assignments only for partitions that were manually
> assigned
> > >> > > > replicas
> > >> > > > >>> in the
> > >> > > > >>> > > request that is being validated. Similarly, for alter
> topic
> > >> > > > >>> validation, it
> > >> > > > >>> > > would be nice to make it more clear for the user what
> has
> > >> been
> > >> > > > >>> changed. I
> > >> > > > >>> > > remember that you already raised that point earlier by
> > >> > comparing
> > >> > > > >>> current
> > >> > > > >>> > > proposed API with having separate methods for each
> specific
> > >> > > > command.
> > >> > > > >>> > > However, I agree that it will make it harder to change
> the
> > >> > > > interface
> > >> > > > >>> in the
> > >> > > > >>> > > future.
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > Could we explore the option of pushing methods that are
> > >> > currently
> > >> > > > in
> > >> > > > >>> > > TopicState to CreateTopicRequest and AlterTopicRequest?
> > >> > > TopicState
> > >> > > > >>> will
> > >> > > > >>> > > still be used for requesting current topic state via
> > >> > > ClusterState.
> > >> > > > >>> > >
> > >> > > > >>> > > Something like:
> > >> > > > >>> > >
> > >> > > > >>> > > interface CreateTopicRequest extends
> > >> AbstractRequestMetadata {
> > >> > > > >>> > >
> > >> > > > >>> > > // requested number of partitions or if manual
> assignment
> > >> is
> > >> > > > given,
> > >> > > > >>> > > number of partitions in the assignment
> > >> > > > >>> > >
> > >> > > > >>> > > int numPartitions();
> > >> > > > >>> > >
> > >> > > > >>> > > // requested replication factor, or if manual
> assignment
> > >> is
> > >> > > > given,
> > >> > > > >>> > > number of replicas in assignment for partition 0
> > >> > > > >>> > >
> > >> > > > >>> > > short replicationFactor();
> > >> > > > >>> > >
> > >> > > > >>> > > // replica assignment requested by the client, or null
> if
> > >> > > > >>> assignment is
> > >> > > > >>> > > auto-generated
> > >> > > > >>> > >
> > >> > > > >>> > > map<Integer, List<Integer>> manualReplicaAssignment();
> > >> > > > >>> > >
> > >> > > > >>> > > map<String, String> configs();
> > >> > > > >>> > >
> > >> > > > >>> > > }
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > interface AlterTopicRequest extends
> AbstractRequestMetadata
> > >> {
> > >> > > > >>> > >
> > >> > > > >>> > > // updated topic configs, or null if not changed
> > >> > > > >>> > >
> > >> > > > >>> > > map<String, String> updatedConfigs();
> > >> > > > >>> > >
> > >> > > > >>> > > // proposed replica assignment in this request, or
> null.
> > >> For
> > >> > > > >>> adding new
> > >> > > > >>> > > partitions request, this is proposed replica assignment
> for
> > >> new
> > >> > > > >>> partitions.
> > >> > > > >>> > > For replica re-assignment case, this is proposed new
> > >> > assignment.
> > >> > > > >>> > >
> > >> > > > >>> > > map<Integer, List<Integer>>
> proposedReplicaAssignment();
> > >> > > > >>> > >
> > >> > > > >>> > > // new number of partitions (due to
> increase/decrease), or
> > >> > null
> > >> > > > if
> > >> > > > >>> > > number of partitions not changed
> > >> > > > >>> > >
> > >> > > > >>> > > Integer updatedNumPartitions()
> > >> > > > >>> > >
> > >> > > > >>> > > }
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > I did not spend much time on my AlterTopicRequest
> interface
> > >> > > > >>> proposal, but
> > >> > > > >>> > > the idea is basically to return only the parts which
> were
> > >> > > changed.
> > >> > > > >>> The
> > >> > > > >>> > > advantage of this approach over having separate methods
> for
> > >> > each
> > >> > > > >>> specific
> > >> > > > >>> > > alter topic request is that it is more flexible for
> future
> > >> > mixing
> > >> > > > of
> > >> > > > >>> what
> > >> > > > >>> > > can be updated in the topic state.
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > What do you think?
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > Thanks,
> > >> > > > >>> > >
> > >> > > > >>> > > Anna
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>> > > On Mon, Oct 9, 2017 at 1:39 AM, Tom Bentley <
> > >> > > t.j.bentley@gmail.com
> > >> > > > >
> > >> > > > >>> wrote:
> > >> > > > >>> > >
> > >> > > > >>> > >> I've added RequestedTopicState, as discussed in my last
> > >> email.
> > >> > > > >>> > >>
> > >> > > > >>> > >> I've also added a paragraph to the migration plan
> about old
> > >> > > > clients
> > >> > > > >>> making
> > >> > > > >>> > >> policy-violating delete topics or delete records
> request.
> > >> > > > >>> > >>
> > >> > > > >>> > >> If no further comments a forthcoming in the next day
> or two
> > >> > > then I
> > >> > > > >>> will
> > >> > > > >>> > >> start a vote.
> > >> > > > >>> > >>
> > >> > > > >>> > >> Thanks,
> > >> > > > >>> > >>
> > >> > > > >>> > >> Tom
> > >> > > > >>> > >>
> > >> > > > >>> > >> On 5 October 2017 at 12:41, Tom Bentley <
> > >> > t.j.bentley@gmail.com>
> > >> > > > >>> wrote:
> > >> > > > >>> > >>
> > >> > > > >>> > >> > I'd like to raise a somewhat subtle point about how
> the
> > >> > > proposed
> > >> > > > >>> API
> > >> > > > >>> > >> > should behave.
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > The current CreateTopicPolicy gets passed either the
> > >> request
> > >> > > > >>> partition
> > >> > > > >>> > >> > count and replication factor, or the requested
> > >> assignment.
> > >> > So
> > >> > > if
> > >> > > > >>> the
> > >> > > > >>> > >> > request had specified partition count and replication
> > >> > factor,
> > >> > > > the
> > >> > > > >>> policy
> > >> > > > >>> > >> > sees a null replicaAssignments(). Likewise if the
> request
> > >> > > > >>> specified a
> > >> > > > >>> > >> > replica assignment the policy would get back null
> from
> > >> > > > >>> numPartitions()
> > >> > > > >>> > >> and
> > >> > > > >>> > >> > replicationFactor().
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > These semantics mean the policy can't reject an
> > >> assignment
> > >> > > that
> > >> > > > >>> happened
> > >> > > > >>> > >> > to be auto-generated (or rather, it's obvious to the
> > >> policy
> > >> > > that
> > >> > > > >>> the
> > >> > > > >>> > >> > assignment is auto generated, because it can't see
> such
> > >> > > > >>> assignments),
> > >> > > > >>> > >> > though it can reject a request because the
> assignment was
> > >> > > > >>> > >> auto-generated,
> > >> > > > >>> > >> > or vice versa.
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > Retaining these semantics makes the TopicState less
> > >> > symmetric
> > >> > > > >>> between
> > >> > > > >>> > >> it's
> > >> > > > >>> > >> > use in requestedState() and the current state
> available
> > >> from
> > >> > > the
> > >> > > > >>> > >> > ClusterState, and also less symmetric between its
> use for
> > >> > > > >>> createTopic()
> > >> > > > >>> > >> and
> > >> > > > >>> > >> > for alterTopic(). This can make it harder to write a
> > >> policy.
> > >> > > For
> > >> > > > >>> > >> example,
> > >> > > > >>> > >> > if I want the policy "the number of partitions must
> be <
> > >> > 100",
> > >> > > > if
> > >> > > > >>> the
> > >> > > > >>> > >> > requestedState().numPartitions() can be null I need
> to
> > >> cope
> > >> > > with
> > >> > > > >>> that
> > >> > > > >>> > >> > and figure it out from inspecting the
> > >> > replicasAssignments().
> > >> > > It
> > >> > > > >>> would
> > >> > > > >>> > >> be
> > >> > > > >>> > >> > much better for the policy writer to just be able to
> > >> write:
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > if (request.requestedState().numPartitions() >=
> 100)
> > >> > > > >>> > >> > throw new
> PolicyViolationException("#partitions
> > >> must
> > >> > > be
> > >> > > > <
> > >> > > > >>> 100")
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > An alternative would be to keep the symmetry (and
> thus
> > >> > > > >>> > >> TopicState.replicasAssignments()
> > >> > > > >>> > >> > would never return null, and
> TopicState.numPartitions()
> > >> and
> > >> > > > >>> > >> > TopicState.replicationFactor() could each be
> primitives),
> > >> > but
> > >> > > > >>> expose the
> > >> > > > >>> > >> > auto-generatedness of the replicaAssignments()
> > >> explicitly,
> > >> > > > >>> perhaps by
> > >> > > > >>> > >> using
> > >> > > > >>> > >> > a subtype of TopicState for the return type of
> > >> > > requestedState():
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > interface RequestedTopicState extends TopicState
> {
> > >> > > > >>> > >> > /**
> > >> > > > >>> > >> > * True if the {@link
> > >> > > TopicState#replicasAssignments()}
> > >> > > > >>> > >> > * in this request we generated by the
> broker,
> > >> false
> > >> > > if
> > >> > > > >>> > >> > * they were explicitly requested by the
> client.
> > >> > > > >>> > >> > */
> > >> > > > >>> > >> > boolean generatedReplicaAssignments();
> > >> > > > >>> > >> > }
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > Thoughts?
> > >> > > > >>> > >> >
> > >> > > > >>> > >> > On 4 October 2017 at 11:06, Tom Bentley <
> > >> > > t.j.bentley@gmail.com>
> > >> > > > >>> wrote:
> > >> > > > >>> > >> >
> > >> > > > >>> > >> >> Good point. Then I guess I can do those items too. I
> > >> would
> > >> > > also
> > >> > > > >>> need to
> > >> > > > >>> > >> >> do the same changes for DeleteRecordsRequest and
> > >> Response.
> > >> > > > >>> > >> >>
> > >> > > > >>> > >> >> On 4 October 2017 at 10:37, Ismael Juma <
> > >> ismael@juma.me.uk
> > >> > >
> > >> > > > >>> wrote:
> > >> > > > >>> > >> >>
> > >> > > > >>> > >> >>> Those two points are related to policies in the
> > >> following
> > >> > > > sense:
> > >> > > > >>> > >> >>>
> > >> > > > >>> > >> >>> 1. A policy that can't send errors to clients is
> much
> > >> less
> > >> > > > >>> useful
> > >> > > > >>> > >> >>> 2. Testing policies is much easier with
> `validateOnly`
> > >> > > > >>> > >> >>>
> > >> > > > >>> > >> >>> Ismael
> > >> > > > >>> > >> >>>
> > >> > > > >>> > >> >>> On Wed, Oct 4, 2017 at 9:20 AM, Tom Bentley <
> > >> > > > >>> t.j.bentley@gmail.com>
> > >> > > > >>> > >> >>> wrote:
> > >> > > > >>> > >> >>>
> > >> > > > >>> > >> >>> > Thanks Edoardo,
> > >> > > > >>> > >> >>> >
> > >> > > > >>> > >> >>> > I've added that motivation to the KIP.
> > >> > > > >>> > >> >>> >
> > >> > > > >>> > >> >>> > KIP-201 doesn't address two points raised in
> KIP-170:
> > >> > > > Adding a
> > >> > > > >>> > >> >>> > validationOnly flag to
> > >> > > > >>> > >> >>> > DeleteTopicRequest and adding an error message to
> > >> > > > >>> > >> DeleteTopicResponse.
> > >> > > > >>> > >> >>> > Since those are not policy-related I think
> they're
> > >> best
> > >> > > left
> > >> > > > >>> out of
> > >> > > > >>> > >> >>> > KIP-201. I suppose it is up to you and Mickael
> > >> whether
> > >> > to
> > >> > > > >>> narrow the
> > >> > > > >>> > >> >>> scope
> > >> > > > >>> > >> >>> > of KIP-170 to address those points.
> > >> > > > >>> > >> >>> >
> > >> > > > >>> > >> >>> > Thanks again,
> > >> > > > >>> > >> >>> >
> > >> > > > >>> > >> >>> > Tom
> > >> > > > >>> > >> >>> >
> > >> > > > >>> > >> >>> > On 4 October 2017 at 08:20, Edoardo Comar <
> > >> > > > ECOMAR@uk.ibm.com>
> > >> > > > >>> > >> wrote:
> > >> > > > >>> > >> >>> >
> > >> > > > >>> > >> >>> > > Thanks Tom,
> > >> > > > >>> > >> >>> > > looks got to me and KIP-201 could supersede
> KIP-170
> > >> > > > >>> > >> >>> > > but could you please add a missing motivation
> > >> bullet
> > >> > > that
> > >> > > > >>> was
> > >> > > > >>> > >> behind
> > >> > > > >>> > >> >>> > > KIP-170:
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > introducing ClusterState to allow validation of
> > >> > > > >>> create/alter topic
> > >> > > > >>> > >> >>> > request
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > not just against the request metadata but also
> > >> > > > >>> > >> >>> > > against the current amount of resources already
> > >> used
> > >> > in
> > >> > > > the
> > >> > > > >>> > >> cluster
> > >> > > > >>> > >> >>> (eg
> > >> > > > >>> > >> >>> > > number of partitions).
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > thanks
> > >> > > > >>> > >> >>> > > Edo
> > >> > > > >>> > >> >>> > >
> --------------------------------------------------
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > Edoardo Comar
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > IBM Message Hub
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > IBM UK Ltd, Hursley Park, SO21 2JN
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > From: Tom Bentley <t....@gmail.com>
> > >> > > > >>> > >> >>> > > To: dev@kafka.apache.org
> > >> > > > >>> > >> >>> > > Date: 02/10/2017 15:15
> > >> > > > >>> > >> >>> > > Subject: Re: [DISCUSS] KIP-201:
> > >> Rationalising
> > >> > > > Policy
> > >> > > > >>> > >> >>> interfaces
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > Hi All,
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > I've updated KIP-201 again so there is now a
> single
> > >> > > policy
> > >> > > > >>> > >> interface
> > >> > > > >>> > >> >>> (and
> > >> > > > >>> > >> >>> > > thus a single key by which to configure it) for
> > >> topic
> > >> > > > >>> creation,
> > >> > > > >>> > >> >>> > > modification, deletion and record deletion,
> which
> > >> each
> > >> > > > have
> > >> > > > >>> their
> > >> > > > >>> > >> own
> > >> > > > >>> > >> >>> > > validation method.
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > There are still a few loose ends:
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > 1. I currently propose validateAlterTopic(),
> but it
> > >> > > would
> > >> > > > be
> > >> > > > >>> > >> >>> possible to
> > >> > > > >>> > >> >>> > > be
> > >> > > > >>> > >> >>> > > more fine grained about this:
> > >> validateAlterConfig(),
> > >> > > > >>> > >> >>> validAddPartitions()
> > >> > > > >>> > >> >>> > > and validateReassignPartitions(), for example.
> > >> > Obviously
> > >> > > > >>> this
> > >> > > > >>> > >> >>> results in
> > >> > > > >>> > >> >>> > a
> > >> > > > >>> > >> >>> > > policy method per operation, and makes it more
> > >> clear
> > >> > > what
> > >> > > > >>> is being
> > >> > > > >>> > >> >>> > > changed.
> > >> > > > >>> > >> >>> > > I guess the down side is its more work for
> > >> > implementer,
> > >> > > > and
> > >> > > > >>> > >> >>> potentially
> > >> > > > >>> > >> >>> > > makes it harder to change the interface in the
> > >> future.
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > 2. A couple of TODOs about what the TopicState
> > >> > interface
> > >> > > > >>> should
> > >> > > > >>> > >> >>> return
> > >> > > > >>> > >> >>> > > when
> > >> > > > >>> > >> >>> > > a topic's partitions are being reassigned.
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > Your thoughts on these or any other points are
> > >> > welcome.
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > Thanks,
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > Tom
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > On 27 September 2017 at 11:45, Paolo Patierno <
> > >> > > > >>> ppatierno@live.com
> > >> > > > >>> > >> >
> > >> > > > >>> > >> >>> > wrote:
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > > Hi Ismael,
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > 1. I don't have a real requirement now but
> > >> > > "deleting"
> > >> > > > >>> is an
> > >> > > > >>> > >> >>> > operation
> > >> > > > >>> > >> >>> > > > that could be really dangerous so it's always
> > >> better
> > >> > > > >>> having a
> > >> > > > >>> > >> way
> > >> > > > >>> > >> >>> for
> > >> > > > >>> > >> >>> > > > having more control on that. I know that we
> have
> > >> the
> > >> > > > >>> authorizer
> > >> > > > >>> > >> >>> used
> > >> > > > >>> > >> >>> > for
> > >> > > > >>> > >> >>> > > > that (delete on topic) but fine grained
> control
> > >> > could
> > >> > > be
> > >> > > > >>> better
> > >> > > > >>> > >> >>> (even
> > >> > > > >>> > >> >>> > > > already happens for topic deletion).
> > >> > > > >>> > >> >>> > > > 2. I know about the problem of restarting
> > >> broker
> > >> > > due
> > >> > > > to
> > >> > > > >>> > >> changes
> > >> > > > >>> > >> >>> on
> > >> > > > >>> > >> >>> > > > policies but what do you mean by doing that
> on
> > >> the
> > >> > > > >>> clients ?
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > Paolo Patierno
> > >> > > > >>> > >> >>> > > > Senior Software Engineer (IoT) @ Red Hat
> > >> > > > >>> > >> >>> > > > Microsoft MVP on Azure & IoT
> > >> > > > >>> > >> >>> > > > Microsoft Azure Advisor
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > Twitter : @ppatierno<
> > >> > > > >>> > >> >>> > >
> > >> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter
> > >> > > > >>> .
> > >> > > > >>> > >> >>> > >
> com_ppatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> > >> > > > >>> > >> >>> > >
> > >> yh7dKgV77XtsUnJ9Rab1gheY&s=43hzTLEDKw2v5Vh0zwkMTaaKD-
> > >> > > > >>> > >> >>> > HdJD8d_F4-Bsw25-Y&e=
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > Linkedin : paolopatierno<
> > >> > > > >>> > >> >>> > >
> > >> > https://urldefense.proofpoint.com/v2/url?u=http-3A__it.
> > >> > > > >>> > >> >>> > >
> > >> > > > linkedin.com_in_paolopatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1
> > >> > > > >>> > >> ZOg&r=
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> > >> > > > >>> > >> >>> > >
> > >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Ig0N7Nwf9EHfTJ2pH3jRM1JIdlzXw6
> > >> > > > >>> > >> >>> > R5Drocu0TMRLk&e=
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > Blog : DevExperience<
> > >> > > > >>> > >> >>> > >
> > >> https://urldefense.proofpoint.com/v2/url?u=http-3A__
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> paolopatierno.wordpress.com_&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> > >> > > > >>> > >> >>> > >
> > >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Tc9NrTtG2GP7-zRjOHkXHfYI0rncO8_
> > >> > > > >>> > >> >>> > jKpedna692z4&e=
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > ________________________________
> > >> > > > >>> > >> >>> > > > From: ismaelj@gmail.com <is...@gmail.com>
> on
> > >> > behalf
> > >> > > > of
> > >> > > > >>> Ismael
> > >> > > > >>> > >> >>> Juma <
> > >> > > > >>> > >> >>> > > > ismael@juma.me.uk>
> > >> > > > >>> > >> >>> > > > Sent: Wednesday, September 27, 2017 10:30 AM
> > >> > > > >>> > >> >>> > > > To: dev@kafka.apache.org
> > >> > > > >>> > >> >>> > > > Subject: Re: [DISCUSS] KIP-201: Rationalising
> > >> Policy
> > >> > > > >>> interfaces
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > A couple of questions:
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > 1. Is this a concrete requirement from a
> user or
> > >> is
> > >> > it
> > >> > > > >>> > >> >>> hypothetical?
> > >> > > > >>> > >> >>> > > > 2. You sure you would want to do this in the
> > >> broker
> > >> > > > >>> instead of
> > >> > > > >>> > >> the
> > >> > > > >>> > >> >>> > > clients?
> > >> > > > >>> > >> >>> > > > It's worth remembering that updating broker
> > >> policies
> > >> > > > >>> involves a
> > >> > > > >>> > >> >>> rolling
> > >> > > > >>> > >> >>> > > > restart of the cluster, so it's not the right
> > >> place
> > >> > > for
> > >> > > > >>> things
> > >> > > > >>> > >> that
> > >> > > > >>> > >> >>> > > change
> > >> > > > >>> > >> >>> > > > frequently.
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > Ismael
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > On Wed, Sep 27, 2017 at 11:26 AM, Paolo
> Patierno
> > >> <
> > >> > > > >>> > >> >>> ppatierno@live.com>
> > >> > > > >>> > >> >>> > > > wrote:
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > > Hi Ismael,
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > > > regarding motivations for delete records,
> as I
> > >> > said
> > >> > > > >>> during the
> > >> > > > >>> > >> >>> > > discussion
> > >> > > > >>> > >> >>> > > > > on KIP-204, it gives the possibility to
> avoid
> > >> > > deleting
> > >> > > > >>> > >> messages
> > >> > > > >>> > >> >>> for
> > >> > > > >>> > >> >>> > > > > specific partitions (inside the topic) and
> > >> > starting
> > >> > > > >>> from a
> > >> > > > >>> > >> >>> specific
> > >> > > > >>> > >> >>> > > > offset.
> > >> > > > >>> > >> >>> > > > > I could think on some users solutions where
> > >> they
> > >> > > know
> > >> > > > >>> exactly
> > >> > > > >>> > >> >>> what
> > >> > > > >>> > >> >>> > the
> > >> > > > >>> > >> >>> > > > > partitions means in a specific topic
> (because
> > >> they
> > >> > > are
> > >> > > > >>> using a
> > >> > > > >>> > >> >>> custom
> > >> > > > >>> > >> >>> > > > > partitioner on the producer side) so they
> know
> > >> > what
> > >> > > > >>> kind of
> > >> > > > >>> > >> >>> messages
> > >> > > > >>> > >> >>> > > are
> > >> > > > >>> > >> >>> > > > > inside a partition allowing to delete them
> but
> > >> not
> > >> > > the
> > >> > > > >>> others.
> > >> > > > >>> > >> >>> In
> > >> > > > >>> > >> >>> > > such a
> > >> > > > >>> > >> >>> > > > > policy a user could also check the
> timestamp
> > >> > related
> > >> > > > to
> > >> > > > >>> the
> > >> > > > >>> > >> >>> offset
> > >> > > > >>> > >> >>> > for
> > >> > > > >>> > >> >>> > > > > allowing or not deletion on time base.
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > > > Paolo Patierno
> > >> > > > >>> > >> >>> > > > > Senior Software Engineer (IoT) @ Red Hat
> > >> > > > >>> > >> >>> > > > > Microsoft MVP on Azure & IoT
> > >> > > > >>> > >> >>> > > > > Microsoft Azure Advisor
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > > > Twitter : @ppatierno<
> > >> > > > >>> > >> >>> > >
> > >> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter
> > >> > > > >>> .
> > >> > > > >>> > >> >>> > >
> com_ppatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> > >> > > > >>> > >> >>> > >
> > >> yh7dKgV77XtsUnJ9Rab1gheY&s=43hzTLEDKw2v5Vh0zwkMTaaKD-
> > >> > > > >>> > >> >>> > HdJD8d_F4-Bsw25-Y&e=
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > > Linkedin : paolopatierno<
> > >> > > > >>> > >> >>> > >
> > >> > https://urldefense.proofpoint.com/v2/url?u=http-3A__it.
> > >> > > > >>> > >> >>> > >
> > >> > > > linkedin.com_in_paolopatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1
> > >> > > > >>> > >> ZOg&r=
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> > >> > > > >>> > >> >>> > >
> > >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Ig0N7Nwf9EHfTJ2pH3jRM1JIdlzXw6
> > >> > > > >>> > >> >>> > R5Drocu0TMRLk&e=
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > > Blog : DevExperience<
> > >> > > > >>> > >> >>> > >
> > >> https://urldefense.proofpoint.com/v2/url?u=http-3A__
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> paolopatierno.wordpress.com_&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> > >> > > > >>> > >> >>> > >
> > >> > > > >>>
> > >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> > >> > > > >>> > >> >>> > >
> > >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Tc9NrTtG2GP7-zRjOHkXHfYI0rncO8_
> > >> > > > >>> > >> >>> > jKpedna692z4&e=
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > > > ________________________________
> > >> > > > >>> > >> >>> > > > > From: ismaelj@gmail.com <is...@gmail.com>
> on
> > >> > > behalf
> > >> > > > >>> of
> > >> > > > >>> > >> Ismael
> > >> > > > >>> > >> >>> > Juma <
> > >> > > > >>> > >> >>> > > > > ismael@juma.me.uk>
> > >> > > > >>> > >> >>> > > > > Sent: Wednesday, September 27, 2017 10:18
> AM
> > >> > > > >>> > >> >>> > > > > To: dev@kafka.apache.org
> > >> > > > >>> > >> >>> > > > > Subject: Re: [DISCUSS] KIP-201:
> Rationalising
> > >> > Policy
> > >> > > > >>> > >> interfaces
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > > > 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 <
> > >> > > > >>> > >> ismael@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....@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 <
> > >> > > > >>> > >> >>> ECOMAR@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.bentley@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
> > >> > > > >>> > >> >>> > > > > >> >
> > >> > > > >>> > >> >>> > > > > >>
> > >> > > > >>> > >> >>> > > > > >
> > >> > > > >>> > >> >>> > > > > >
> > >> > > > >>> > >> >>> > > > >
> > >> > > > >>> > >> >>> > > >
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> > > 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
> > >> > > > >>> > >> >>> > >
> > >> > > > >>> > >> >>> >
> > >> > > > >>> > >> >>>
> > >> > > > >>> > >> >>
> > >> > > > >>> > >> >>
> > >> > > > >>> > >> >
> > >> > > > >>> > >>
> > >> > > > >>> > >
> > >> > > > >>> > >
> > >> > > > >>>
> > >> > > > >>
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
>
Re: [DISCUSS] KIP-201: Rationalising Policy interfaces
Posted by Mickael Maison <mi...@gmail.com>.
Hi Tom,
Thanks for following up on this KIP. This is a great improvement that
will make policies more powerful and at the same time easier to
manage.
I just have one question:
In AbstractRequestMetadata.principal() javadoc, it says the principal
will be "null" for non authenticated session. Can't we just have the
default Principal for the Session instead of null? It's possible to
have Principals for PLAINTEXT sessions or use the default ANONYMOUS
Principal.
Thanks
On Mon, Aug 12, 2019 at 10:52 AM Tom Bentley <tb...@redhat.com> wrote:
>
> Hi folks,
>
> As far as I can see the motivation for KIP-201 is still valid, and as far
> as I'm aware the changes I made to the KIP back in April addressed the
> previous comments. Since the issue still needs to be addressed I intend to
> start another vote thread in the near future, but before I do I thought I'd
> check whether anyone has any more comments. So please let me know any
> feedback for this KIP.
>
> Many thanks,
>
> Tom
>
> On Fri, Apr 12, 2019 at 10:46 AM Tom Bentley <tb...@redhat.com> wrote:
>
> > Hi Rajini,
> >
> > I've made a number of changes to the KIP.
> >
> > 1. I've added RequestedTopicState.requestedConfigs(). This is obviously
> > unrelated to supporting alter broker, but I think it goes some way to
> > addressing one of the points Anna made last year.
> > Anna, wdyt?
> >
> > 2. I've added BrokerState, RequestedBrokerState and
> > BrokerManagementPolicy. These are largely similar to the interfaces for
> > topic management, but the lifecycle of a BrokerManagementPolicy needs to be
> > different.
> >
> > That's because a BrokerManagementPolicy ought to be Configurable with the
> > broker config, but obviously the broker config can change. Because a
> > cluster-scoped config might be changed via a different broker we need to
> > hook into the Zookeeper change notification on the broker configs to
> > instantiate a new BrokerManagementPolicy when broker policy changes. We'd
> > need to cope with policy implementation change happening concurrently with
> > policy enforcement.
> > And technically there's a race here: Sending changes to cluster-scoped
> > configs to multiple brokers could result in non-deterministic policy
> > enforcement.
> >
> > One way to avoid that would be to require changes to cluster-scoped
> > configs to be sent to the controller.
> > This complexity is annoying because it seems likely that many policy
> > implementations won't _actually_ depend on the broker config.
> >
> > Thoughts?
> >
> > Kind regards,
> >
> > Tom
> >
> > On Wed, Apr 10, 2019 at 9:48 AM Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> >> Thanks Tom.
> >>
> >> Once you have updated the KIP to support broker config updates, it may be
> >> good to start a new vote thread since the other one is quite old and
> >> perhaps the KIP has changed since then.
> >>
> >>
> >> On Wed, Apr 10, 2019 at 3:58 AM Tom Bentley <tb...@redhat.com> wrote:
> >>
> >> > Hi Rajini,
> >> >
> >> > I'd be happy to do that. I'll try to get it done in the next few days.
> >> >
> >> > Although there's been quite a lot of interest this, the vote thread
> >> never
> >> > got any binding +1, so it's been stuck in limbo for a long time. It
> >> would
> >> > be great to get this moving again.
> >> >
> >> > Kind regards,
> >> >
> >> > Tom
> >> >
> >> > On Tue, Apr 9, 2019 at 3:04 PM Rajini Sivaram <ra...@gmail.com>
> >> > wrote:
> >> >
> >> > > Hi Tom,
> >> > >
> >> > > Are you planning to extend this KIP to also include dynamic broker
> >> config
> >> > > update (currently covered under AlterConfigPolicy)?
> >> > >
> >> > > May be worth sending another note to make progress on this KIP since
> >> it
> >> > has
> >> > > been around a while and reading through the threads, it looks like
> >> there
> >> > > has been a lot of interest in it.
> >> > >
> >> > > Thank you,
> >> > >
> >> > > Rajini
> >> > >
> >> > >
> >> > > On Wed, Jan 9, 2019 at 11:25 AM Tom Bentley <t....@gmail.com>
> >> > wrote:
> >> > >
> >> > > > Hi Anna and Mickael,
> >> > > >
> >> > > > Anna, did you have any comments about the points I made?
> >> > > >
> >> > > > Mickael, we really need the vote to be passed before there's even
> >> any
> >> > > work
> >> > > > to do. With the exception of Ismael, the KIP didn't seem to get the
> >> > > > attention of any of the other committers.
> >> > > >
> >> > > > Kind regards,
> >> > > >
> >> > > > Tom
> >> > > >
> >> > > > On Thu, 13 Dec 2018 at 18:11, Tom Bentley <t....@gmail.com>
> >> > wrote:
> >> > > >
> >> > > > > Hi Anna,
> >> > > > >
> >> > > > > Firstly, let me apologise again about having missed your previous
> >> > > emails
> >> > > > > about this.
> >> > > > >
> >> > > > > Thank you for the feedback. You raise some valid points about
> >> > > ambiguity.
> >> > > > > The problem with pulling the metadata into CreateTopicRequest and
> >> > > > > AlterTopicRequest is that you lose the benefit of being able to
> >> eaily
> >> > > > write
> >> > > > > a common policy across creation and alter cases. For example, with
> >> > the
> >> > > > > proposed design the policy maker could write code like this
> >> (forgive
> >> > my
> >> > > > > pseudo-Java)
> >> > > > >
> >> > > > > public void validateCreateTopic(requestMetadata, ...) {
> >> > > > > commonPolicy(requestMetadata.requestedState());
> >> > > > > }
> >> > > > >
> >> > > > > public void validateAlterTopic(requestMetadata, ...) {
> >> > > > > commonPolicy(requestMetadata.requestedState());
> >> > > > > }
> >> > > > >
> >> > > > > private void commonPolicy(RequestedTopicState requestedState) {
> >> > > > > // ...
> >> > > > > }
> >> > > > >
> >> > > > > I think that's an important feature of the API because (I think)
> >> very
> >> > > > > often the policy maker is interested in defining the universe of
> >> > > > prohibited
> >> > > > > configurations without really caring about whether the request is
> >> a
> >> > > > create
> >> > > > > or an alter. Having a single RequestedTopicState for both create
> >> and
> >> > > > > alter means they can do that trivially in one place. Having
> >> different
> >> > > > > methods in the two Request classes prevents this and forces the
> >> > policy
> >> > > > > maker to pick apart the different requestState objects before
> >> calling
> >> > > any
> >> > > > > common method(s).
> >> > > > >
> >> > > > > I think my intention at the time (and it's many months ago now,
> >> so I
> >> > > > might
> >> > > > > not have remembered fully) was that RequestedTopicState would
> >> > basically
> >> > > > > represent what the topic would look like after the requested
> >> changes
> >> > > were
> >> > > > > applied (I accept this isn't how it's Javadoc'd in the KIP),
> >> rather
> >> > > than
> >> > > > > representing the request itself. Thus if the request changed the
> >> > > > assignment
> >> > > > > of some of the partitions and the policy maker was interested in
> >> > > > precisely
> >> > > > > which partitions would be changed, and how, they would indeed
> >> have to
> >> > > > > compute that for themselves by looking up the current topic state
> >> > from
> >> > > > the
> >> > > > > cluster state and seeing how they differed. Indeed they'd have to
> >> do
> >> > > this
> >> > > > > diff even to figure out that the user was requesting a change to
> >> the
> >> > > > topic
> >> > > > > assigned (or similarly for topic config, etc). To me this is
> >> > acceptable
> >> > > > > because I think most people writing such policies are just
> >> interested
> >> > > in
> >> > > > > defining what is not allowed, so giving them a representation of
> >> the
> >> > > > > proposed topic state which they can readily check against is the
> >> most
> >> > > > > direct API. In this interpretation generatedReplicaAssignment()
> >> would
> >> > > > > just be some extra metadata annotating whether any difference
> >> between
> >> > > the
> >> > > > > current and proposed states was directly from the user, or
> >> generated
> >> > on
> >> > > > the
> >> > > > > broker. You're right that it's ambiguous when the request didn't
> >> > > actually
> >> > > > > change the assignment but I didn't envisage policy makers using it
> >> > > except
> >> > > > > when the assignments differed anyway. To me it would be
> >> acceptable to
> >> > > > > Javadoc this.
> >> > > > >
> >> > > > > Given this interpretation of RequestedTopicState as "what the
> >> topic
> >> > > would
> >> > > > > look like after the requested changes were applied" can you see
> >> any
> >> > > other
> >> > > > > problems with the proposal? Or do you have use cases where the
> >> policy
> >> > > > maker
> >> > > > > is more interested in what the request is changing?
> >> > > > >
> >> > > > > Kind regards,
> >> > > > >
> >> > > > > Tom
> >> > > > >
> >> > > > > On Fri, 7 Dec 2018 at 08:41, Tom Bentley <t....@gmail.com>
> >> > > wrote:
> >> > > > >
> >> > > > >> Hi Anna and Mickael,
> >> > > > >>
> >> > > > >> Sorry for remaining silent on this for so long. I should have
> >> time
> >> > to
> >> > > > >> look at this again next week.
> >> > > > >>
> >> > > > >> Kind regards,
> >> > > > >>
> >> > > > >> Tom
> >> > > > >>
> >> > > > >> On Mon, 3 Dec 2018 at 10:11, Mickael Maison <
> >> > mickael.maison@gmail.com
> >> > > >
> >> > > > >> wrote:
> >> > > > >>
> >> > > > >>> Hi Tom,
> >> > > > >>>
> >> > > > >>> This is a very interesting KIP. If you are not going to continue
> >> > > > >>> working on it, would it be ok for us to grab it and complete it?
> >> > > > >>> Thanks
> >> > > > >>> On Thu, Jun 14, 2018 at 7:06 PM Anna Povzner <anna@confluent.io
> >> >
> >> > > > wrote:
> >> > > > >>> >
> >> > > > >>> > Hi Tom,
> >> > > > >>> >
> >> > > > >>> > Just wanted to check what you think about the comments I made
> >> in
> >> > my
> >> > > > >>> last
> >> > > > >>> > message. I think this KIP is a big improvement to our current
> >> > > policy
> >> > > > >>> > interfaces, and really hope we can get this KIP in.
> >> > > > >>> >
> >> > > > >>> > Thanks,
> >> > > > >>> > Anna
> >> > > > >>> >
> >> > > > >>> > On Thu, May 31, 2018 at 3:29 PM, Anna Povzner <
> >> anna@confluent.io
> >> > >
> >> > > > >>> wrote:
> >> > > > >>> >
> >> > > > >>> > > Hi Tom,
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > Thanks for the KIP. I am aware that the voting thread was
> >> > > started,
> >> > > > >>> but
> >> > > > >>> > > wanted to discuss couple of concerns here first.
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > I think the coupling of
> >> > > > >>> RequestedTopicState#generatedReplicaAssignment()
> >> > > > >>> > > and TopicState#replicasAssignments() does not work well in
> >> case
> >> > > > >>> where the
> >> > > > >>> > > request deals only with a subset of partitions (e.g., add
> >> > > > >>> partitions) or no
> >> > > > >>> > > assignment at all (alter topic config). In particular:
> >> > > > >>> > >
> >> > > > >>> > > 1) Alter topic config use case: There is no replica
> >> assignment
> >> > in
> >> > > > the
> >> > > > >>> > > request, and generatedReplicaAssignment() returning either
> >> > true
> >> > > or
> >> > > > >>> false
> >> > > > >>> > > is both misleading. The user can interpret this as
> >> assignment
> >> > > being
> >> > > > >>> > > generated or provided by the user originally (e.g., on topic
> >> > > > >>> create), while
> >> > > > >>> > > I don’t think we track such thing.
> >> > > > >>> > >
> >> > > > >>> > > 2) On add partitions, we may have manual assignment for new
> >> > > > >>> partitions.
> >> > > > >>> > > What I understood from the KIP,
> >> generatedReplicaAssignment()
> >> > > will
> >> > > > >>> return
> >> > > > >>> > > true or false based on whether new partitions were manually
> >> > > > assigned
> >> > > > >>> or
> >> > > > >>> > > not, while TopicState#replicasAssignments() will return
> >> replica
> >> > > > >>> > > assignments for all partitions. I think it is confusing in a
> >> > way
> >> > > > that
> >> > > > >>> > > assignment of old partitions could be auto-generated but new
> >> > > > >>> partitions are
> >> > > > >>> > > manually assigned.
> >> > > > >>> > >
> >> > > > >>> > > 3) Generalizing #2, suppose in a future, a user can
> >> re-assign
> >> > > > >>> replicas for
> >> > > > >>> > > a set of partitions.
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > One way to address this with minimal changes to proposed
> >> API is
> >> > > to
> >> > > > >>> rename
> >> > > > >>> > > RequestedTopicState#generatedReplicaAssignment() to
> >> > > > >>> RequestedTopicState#manualReplicaAssignment()
> >> > > > >>> > > and change the API behavior and description to : “True if
> >> the
> >> > > > client
> >> > > > >>> > > explicitly provided replica assignments in this request,
> >> which
> >> > > > means
> >> > > > >>> that
> >> > > > >>> > > some or all assignments returned by
> >> > > > TopicState#replicasAssignments()
> >> > > > >>> are
> >> > > > >>> > > explicitly requested by the user”. The user then will have
> >> to
> >> > > diff
> >> > > > >>> > > TopicState#replicasAssignments() from clusterState and
> >> > > TopicState#
> >> > > > >>> > > replicasAssignments() from RequestedTopicState, and assume
> >> > that
> >> > > > >>> > > assignments that are different are manually assigned (if
> >> > > > >>> > > RequestedTopicState#manualReplicaAssignment() returns
> >> true).
> >> > We
> >> > > > will
> >> > > > >>> > > need to clearly document this and it still seems awkward.
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > I think a cleaner way is to make RequestedTopicState to
> >> provide
> >> > > > >>> replica
> >> > > > >>> > > assignments only for partitions that were manually assigned
> >> > > > replicas
> >> > > > >>> in the
> >> > > > >>> > > request that is being validated. Similarly, for alter topic
> >> > > > >>> validation, it
> >> > > > >>> > > would be nice to make it more clear for the user what has
> >> been
> >> > > > >>> changed. I
> >> > > > >>> > > remember that you already raised that point earlier by
> >> > comparing
> >> > > > >>> current
> >> > > > >>> > > proposed API with having separate methods for each specific
> >> > > > command.
> >> > > > >>> > > However, I agree that it will make it harder to change the
> >> > > > interface
> >> > > > >>> in the
> >> > > > >>> > > future.
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > Could we explore the option of pushing methods that are
> >> > currently
> >> > > > in
> >> > > > >>> > > TopicState to CreateTopicRequest and AlterTopicRequest?
> >> > > TopicState
> >> > > > >>> will
> >> > > > >>> > > still be used for requesting current topic state via
> >> > > ClusterState.
> >> > > > >>> > >
> >> > > > >>> > > Something like:
> >> > > > >>> > >
> >> > > > >>> > > interface CreateTopicRequest extends
> >> AbstractRequestMetadata {
> >> > > > >>> > >
> >> > > > >>> > > // requested number of partitions or if manual assignment
> >> is
> >> > > > given,
> >> > > > >>> > > number of partitions in the assignment
> >> > > > >>> > >
> >> > > > >>> > > int numPartitions();
> >> > > > >>> > >
> >> > > > >>> > > // requested replication factor, or if manual assignment
> >> is
> >> > > > given,
> >> > > > >>> > > number of replicas in assignment for partition 0
> >> > > > >>> > >
> >> > > > >>> > > short replicationFactor();
> >> > > > >>> > >
> >> > > > >>> > > // replica assignment requested by the client, or null if
> >> > > > >>> assignment is
> >> > > > >>> > > auto-generated
> >> > > > >>> > >
> >> > > > >>> > > map<Integer, List<Integer>> manualReplicaAssignment();
> >> > > > >>> > >
> >> > > > >>> > > map<String, String> configs();
> >> > > > >>> > >
> >> > > > >>> > > }
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > interface AlterTopicRequest extends AbstractRequestMetadata
> >> {
> >> > > > >>> > >
> >> > > > >>> > > // updated topic configs, or null if not changed
> >> > > > >>> > >
> >> > > > >>> > > map<String, String> updatedConfigs();
> >> > > > >>> > >
> >> > > > >>> > > // proposed replica assignment in this request, or null.
> >> For
> >> > > > >>> adding new
> >> > > > >>> > > partitions request, this is proposed replica assignment for
> >> new
> >> > > > >>> partitions.
> >> > > > >>> > > For replica re-assignment case, this is proposed new
> >> > assignment.
> >> > > > >>> > >
> >> > > > >>> > > map<Integer, List<Integer>> proposedReplicaAssignment();
> >> > > > >>> > >
> >> > > > >>> > > // new number of partitions (due to increase/decrease), or
> >> > null
> >> > > > if
> >> > > > >>> > > number of partitions not changed
> >> > > > >>> > >
> >> > > > >>> > > Integer updatedNumPartitions()
> >> > > > >>> > >
> >> > > > >>> > > }
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > I did not spend much time on my AlterTopicRequest interface
> >> > > > >>> proposal, but
> >> > > > >>> > > the idea is basically to return only the parts which were
> >> > > changed.
> >> > > > >>> The
> >> > > > >>> > > advantage of this approach over having separate methods for
> >> > each
> >> > > > >>> specific
> >> > > > >>> > > alter topic request is that it is more flexible for future
> >> > mixing
> >> > > > of
> >> > > > >>> what
> >> > > > >>> > > can be updated in the topic state.
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > What do you think?
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > Thanks,
> >> > > > >>> > >
> >> > > > >>> > > Anna
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>> > > On Mon, Oct 9, 2017 at 1:39 AM, Tom Bentley <
> >> > > t.j.bentley@gmail.com
> >> > > > >
> >> > > > >>> wrote:
> >> > > > >>> > >
> >> > > > >>> > >> I've added RequestedTopicState, as discussed in my last
> >> email.
> >> > > > >>> > >>
> >> > > > >>> > >> I've also added a paragraph to the migration plan about old
> >> > > > clients
> >> > > > >>> making
> >> > > > >>> > >> policy-violating delete topics or delete records request.
> >> > > > >>> > >>
> >> > > > >>> > >> If no further comments a forthcoming in the next day or two
> >> > > then I
> >> > > > >>> will
> >> > > > >>> > >> start a vote.
> >> > > > >>> > >>
> >> > > > >>> > >> Thanks,
> >> > > > >>> > >>
> >> > > > >>> > >> Tom
> >> > > > >>> > >>
> >> > > > >>> > >> On 5 October 2017 at 12:41, Tom Bentley <
> >> > t.j.bentley@gmail.com>
> >> > > > >>> wrote:
> >> > > > >>> > >>
> >> > > > >>> > >> > I'd like to raise a somewhat subtle point about how the
> >> > > proposed
> >> > > > >>> API
> >> > > > >>> > >> > should behave.
> >> > > > >>> > >> >
> >> > > > >>> > >> > The current CreateTopicPolicy gets passed either the
> >> request
> >> > > > >>> partition
> >> > > > >>> > >> > count and replication factor, or the requested
> >> assignment.
> >> > So
> >> > > if
> >> > > > >>> the
> >> > > > >>> > >> > request had specified partition count and replication
> >> > factor,
> >> > > > the
> >> > > > >>> policy
> >> > > > >>> > >> > sees a null replicaAssignments(). Likewise if the request
> >> > > > >>> specified a
> >> > > > >>> > >> > replica assignment the policy would get back null from
> >> > > > >>> numPartitions()
> >> > > > >>> > >> and
> >> > > > >>> > >> > replicationFactor().
> >> > > > >>> > >> >
> >> > > > >>> > >> > These semantics mean the policy can't reject an
> >> assignment
> >> > > that
> >> > > > >>> happened
> >> > > > >>> > >> > to be auto-generated (or rather, it's obvious to the
> >> policy
> >> > > that
> >> > > > >>> the
> >> > > > >>> > >> > assignment is auto generated, because it can't see such
> >> > > > >>> assignments),
> >> > > > >>> > >> > though it can reject a request because the assignment was
> >> > > > >>> > >> auto-generated,
> >> > > > >>> > >> > or vice versa.
> >> > > > >>> > >> >
> >> > > > >>> > >> > Retaining these semantics makes the TopicState less
> >> > symmetric
> >> > > > >>> between
> >> > > > >>> > >> it's
> >> > > > >>> > >> > use in requestedState() and the current state available
> >> from
> >> > > the
> >> > > > >>> > >> > ClusterState, and also less symmetric between its use for
> >> > > > >>> createTopic()
> >> > > > >>> > >> and
> >> > > > >>> > >> > for alterTopic(). This can make it harder to write a
> >> policy.
> >> > > For
> >> > > > >>> > >> example,
> >> > > > >>> > >> > if I want the policy "the number of partitions must be <
> >> > 100",
> >> > > > if
> >> > > > >>> the
> >> > > > >>> > >> > requestedState().numPartitions() can be null I need to
> >> cope
> >> > > with
> >> > > > >>> that
> >> > > > >>> > >> > and figure it out from inspecting the
> >> > replicasAssignments().
> >> > > It
> >> > > > >>> would
> >> > > > >>> > >> be
> >> > > > >>> > >> > much better for the policy writer to just be able to
> >> write:
> >> > > > >>> > >> >
> >> > > > >>> > >> > if (request.requestedState().numPartitions() >= 100)
> >> > > > >>> > >> > throw new PolicyViolationException("#partitions
> >> must
> >> > > be
> >> > > > <
> >> > > > >>> 100")
> >> > > > >>> > >> >
> >> > > > >>> > >> > An alternative would be to keep the symmetry (and thus
> >> > > > >>> > >> TopicState.replicasAssignments()
> >> > > > >>> > >> > would never return null, and TopicState.numPartitions()
> >> and
> >> > > > >>> > >> > TopicState.replicationFactor() could each be primitives),
> >> > but
> >> > > > >>> expose the
> >> > > > >>> > >> > auto-generatedness of the replicaAssignments()
> >> explicitly,
> >> > > > >>> perhaps by
> >> > > > >>> > >> using
> >> > > > >>> > >> > a subtype of TopicState for the return type of
> >> > > requestedState():
> >> > > > >>> > >> >
> >> > > > >>> > >> > interface RequestedTopicState extends TopicState {
> >> > > > >>> > >> > /**
> >> > > > >>> > >> > * True if the {@link
> >> > > TopicState#replicasAssignments()}
> >> > > > >>> > >> > * in this request we generated by the broker,
> >> false
> >> > > if
> >> > > > >>> > >> > * they were explicitly requested by the client.
> >> > > > >>> > >> > */
> >> > > > >>> > >> > boolean generatedReplicaAssignments();
> >> > > > >>> > >> > }
> >> > > > >>> > >> >
> >> > > > >>> > >> > Thoughts?
> >> > > > >>> > >> >
> >> > > > >>> > >> > On 4 October 2017 at 11:06, Tom Bentley <
> >> > > t.j.bentley@gmail.com>
> >> > > > >>> wrote:
> >> > > > >>> > >> >
> >> > > > >>> > >> >> Good point. Then I guess I can do those items too. I
> >> would
> >> > > also
> >> > > > >>> need to
> >> > > > >>> > >> >> do the same changes for DeleteRecordsRequest and
> >> Response.
> >> > > > >>> > >> >>
> >> > > > >>> > >> >> On 4 October 2017 at 10:37, Ismael Juma <
> >> ismael@juma.me.uk
> >> > >
> >> > > > >>> wrote:
> >> > > > >>> > >> >>
> >> > > > >>> > >> >>> Those two points are related to policies in the
> >> following
> >> > > > sense:
> >> > > > >>> > >> >>>
> >> > > > >>> > >> >>> 1. A policy that can't send errors to clients is much
> >> less
> >> > > > >>> useful
> >> > > > >>> > >> >>> 2. Testing policies is much easier with `validateOnly`
> >> > > > >>> > >> >>>
> >> > > > >>> > >> >>> Ismael
> >> > > > >>> > >> >>>
> >> > > > >>> > >> >>> On Wed, Oct 4, 2017 at 9:20 AM, Tom Bentley <
> >> > > > >>> t.j.bentley@gmail.com>
> >> > > > >>> > >> >>> wrote:
> >> > > > >>> > >> >>>
> >> > > > >>> > >> >>> > Thanks Edoardo,
> >> > > > >>> > >> >>> >
> >> > > > >>> > >> >>> > I've added that motivation to the KIP.
> >> > > > >>> > >> >>> >
> >> > > > >>> > >> >>> > KIP-201 doesn't address two points raised in KIP-170:
> >> > > > Adding a
> >> > > > >>> > >> >>> > validationOnly flag to
> >> > > > >>> > >> >>> > DeleteTopicRequest and adding an error message to
> >> > > > >>> > >> DeleteTopicResponse.
> >> > > > >>> > >> >>> > Since those are not policy-related I think they're
> >> best
> >> > > left
> >> > > > >>> out of
> >> > > > >>> > >> >>> > KIP-201. I suppose it is up to you and Mickael
> >> whether
> >> > to
> >> > > > >>> narrow the
> >> > > > >>> > >> >>> scope
> >> > > > >>> > >> >>> > of KIP-170 to address those points.
> >> > > > >>> > >> >>> >
> >> > > > >>> > >> >>> > Thanks again,
> >> > > > >>> > >> >>> >
> >> > > > >>> > >> >>> > Tom
> >> > > > >>> > >> >>> >
> >> > > > >>> > >> >>> > On 4 October 2017 at 08:20, Edoardo Comar <
> >> > > > ECOMAR@uk.ibm.com>
> >> > > > >>> > >> wrote:
> >> > > > >>> > >> >>> >
> >> > > > >>> > >> >>> > > Thanks Tom,
> >> > > > >>> > >> >>> > > looks got to me and KIP-201 could supersede KIP-170
> >> > > > >>> > >> >>> > > but could you please add a missing motivation
> >> bullet
> >> > > that
> >> > > > >>> was
> >> > > > >>> > >> behind
> >> > > > >>> > >> >>> > > KIP-170:
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > introducing ClusterState to allow validation of
> >> > > > >>> create/alter topic
> >> > > > >>> > >> >>> > request
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > not just against the request metadata but also
> >> > > > >>> > >> >>> > > against the current amount of resources already
> >> used
> >> > in
> >> > > > the
> >> > > > >>> > >> cluster
> >> > > > >>> > >> >>> (eg
> >> > > > >>> > >> >>> > > number of partitions).
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > thanks
> >> > > > >>> > >> >>> > > Edo
> >> > > > >>> > >> >>> > > --------------------------------------------------
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > Edoardo Comar
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > IBM Message Hub
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > IBM UK Ltd, Hursley Park, SO21 2JN
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > From: Tom Bentley <t....@gmail.com>
> >> > > > >>> > >> >>> > > To: dev@kafka.apache.org
> >> > > > >>> > >> >>> > > Date: 02/10/2017 15:15
> >> > > > >>> > >> >>> > > Subject: Re: [DISCUSS] KIP-201:
> >> Rationalising
> >> > > > Policy
> >> > > > >>> > >> >>> interfaces
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > Hi All,
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > I've updated KIP-201 again so there is now a single
> >> > > policy
> >> > > > >>> > >> interface
> >> > > > >>> > >> >>> (and
> >> > > > >>> > >> >>> > > thus a single key by which to configure it) for
> >> topic
> >> > > > >>> creation,
> >> > > > >>> > >> >>> > > modification, deletion and record deletion, which
> >> each
> >> > > > have
> >> > > > >>> their
> >> > > > >>> > >> own
> >> > > > >>> > >> >>> > > validation method.
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > There are still a few loose ends:
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > 1. I currently propose validateAlterTopic(), but it
> >> > > would
> >> > > > be
> >> > > > >>> > >> >>> possible to
> >> > > > >>> > >> >>> > > be
> >> > > > >>> > >> >>> > > more fine grained about this:
> >> validateAlterConfig(),
> >> > > > >>> > >> >>> validAddPartitions()
> >> > > > >>> > >> >>> > > and validateReassignPartitions(), for example.
> >> > Obviously
> >> > > > >>> this
> >> > > > >>> > >> >>> results in
> >> > > > >>> > >> >>> > a
> >> > > > >>> > >> >>> > > policy method per operation, and makes it more
> >> clear
> >> > > what
> >> > > > >>> is being
> >> > > > >>> > >> >>> > > changed.
> >> > > > >>> > >> >>> > > I guess the down side is its more work for
> >> > implementer,
> >> > > > and
> >> > > > >>> > >> >>> potentially
> >> > > > >>> > >> >>> > > makes it harder to change the interface in the
> >> future.
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > 2. A couple of TODOs about what the TopicState
> >> > interface
> >> > > > >>> should
> >> > > > >>> > >> >>> return
> >> > > > >>> > >> >>> > > when
> >> > > > >>> > >> >>> > > a topic's partitions are being reassigned.
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > Your thoughts on these or any other points are
> >> > welcome.
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > Thanks,
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > Tom
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > On 27 September 2017 at 11:45, Paolo Patierno <
> >> > > > >>> ppatierno@live.com
> >> > > > >>> > >> >
> >> > > > >>> > >> >>> > wrote:
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > > Hi Ismael,
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > 1. I don't have a real requirement now but
> >> > > "deleting"
> >> > > > >>> is an
> >> > > > >>> > >> >>> > operation
> >> > > > >>> > >> >>> > > > that could be really dangerous so it's always
> >> better
> >> > > > >>> having a
> >> > > > >>> > >> way
> >> > > > >>> > >> >>> for
> >> > > > >>> > >> >>> > > > having more control on that. I know that we have
> >> the
> >> > > > >>> authorizer
> >> > > > >>> > >> >>> used
> >> > > > >>> > >> >>> > for
> >> > > > >>> > >> >>> > > > that (delete on topic) but fine grained control
> >> > could
> >> > > be
> >> > > > >>> better
> >> > > > >>> > >> >>> (even
> >> > > > >>> > >> >>> > > > already happens for topic deletion).
> >> > > > >>> > >> >>> > > > 2. I know about the problem of restarting
> >> broker
> >> > > due
> >> > > > to
> >> > > > >>> > >> changes
> >> > > > >>> > >> >>> on
> >> > > > >>> > >> >>> > > > policies but what do you mean by doing that on
> >> the
> >> > > > >>> clients ?
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > Paolo Patierno
> >> > > > >>> > >> >>> > > > Senior Software Engineer (IoT) @ Red Hat
> >> > > > >>> > >> >>> > > > Microsoft MVP on Azure & IoT
> >> > > > >>> > >> >>> > > > Microsoft Azure Advisor
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > Twitter : @ppatierno<
> >> > > > >>> > >> >>> > >
> >> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter
> >> > > > >>> .
> >> > > > >>> > >> >>> > > com_ppatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> >> > > > >>> > >> >>> > >
> >> yh7dKgV77XtsUnJ9Rab1gheY&s=43hzTLEDKw2v5Vh0zwkMTaaKD-
> >> > > > >>> > >> >>> > HdJD8d_F4-Bsw25-Y&e=
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > Linkedin : paolopatierno<
> >> > > > >>> > >> >>> > >
> >> > https://urldefense.proofpoint.com/v2/url?u=http-3A__it.
> >> > > > >>> > >> >>> > >
> >> > > > linkedin.com_in_paolopatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1
> >> > > > >>> > >> ZOg&r=
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> >> > > > >>> > >> >>> > >
> >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Ig0N7Nwf9EHfTJ2pH3jRM1JIdlzXw6
> >> > > > >>> > >> >>> > R5Drocu0TMRLk&e=
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > Blog : DevExperience<
> >> > > > >>> > >> >>> > >
> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> paolopatierno.wordpress.com_&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> >> > > > >>> > >> >>> > >
> >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Tc9NrTtG2GP7-zRjOHkXHfYI0rncO8_
> >> > > > >>> > >> >>> > jKpedna692z4&e=
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > ________________________________
> >> > > > >>> > >> >>> > > > From: ismaelj@gmail.com <is...@gmail.com> on
> >> > behalf
> >> > > > of
> >> > > > >>> Ismael
> >> > > > >>> > >> >>> Juma <
> >> > > > >>> > >> >>> > > > ismael@juma.me.uk>
> >> > > > >>> > >> >>> > > > Sent: Wednesday, September 27, 2017 10:30 AM
> >> > > > >>> > >> >>> > > > To: dev@kafka.apache.org
> >> > > > >>> > >> >>> > > > Subject: Re: [DISCUSS] KIP-201: Rationalising
> >> Policy
> >> > > > >>> interfaces
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > A couple of questions:
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > 1. Is this a concrete requirement from a user or
> >> is
> >> > it
> >> > > > >>> > >> >>> hypothetical?
> >> > > > >>> > >> >>> > > > 2. You sure you would want to do this in the
> >> broker
> >> > > > >>> instead of
> >> > > > >>> > >> the
> >> > > > >>> > >> >>> > > clients?
> >> > > > >>> > >> >>> > > > It's worth remembering that updating broker
> >> policies
> >> > > > >>> involves a
> >> > > > >>> > >> >>> rolling
> >> > > > >>> > >> >>> > > > restart of the cluster, so it's not the right
> >> place
> >> > > for
> >> > > > >>> things
> >> > > > >>> > >> that
> >> > > > >>> > >> >>> > > change
> >> > > > >>> > >> >>> > > > frequently.
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > Ismael
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > On Wed, Sep 27, 2017 at 11:26 AM, Paolo Patierno
> >> <
> >> > > > >>> > >> >>> ppatierno@live.com>
> >> > > > >>> > >> >>> > > > wrote:
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > > Hi Ismael,
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > > > regarding motivations for delete records, as I
> >> > said
> >> > > > >>> during the
> >> > > > >>> > >> >>> > > discussion
> >> > > > >>> > >> >>> > > > > on KIP-204, it gives the possibility to avoid
> >> > > deleting
> >> > > > >>> > >> messages
> >> > > > >>> > >> >>> for
> >> > > > >>> > >> >>> > > > > specific partitions (inside the topic) and
> >> > starting
> >> > > > >>> from a
> >> > > > >>> > >> >>> specific
> >> > > > >>> > >> >>> > > > offset.
> >> > > > >>> > >> >>> > > > > I could think on some users solutions where
> >> they
> >> > > know
> >> > > > >>> exactly
> >> > > > >>> > >> >>> what
> >> > > > >>> > >> >>> > the
> >> > > > >>> > >> >>> > > > > partitions means in a specific topic (because
> >> they
> >> > > are
> >> > > > >>> using a
> >> > > > >>> > >> >>> custom
> >> > > > >>> > >> >>> > > > > partitioner on the producer side) so they know
> >> > what
> >> > > > >>> kind of
> >> > > > >>> > >> >>> messages
> >> > > > >>> > >> >>> > > are
> >> > > > >>> > >> >>> > > > > inside a partition allowing to delete them but
> >> not
> >> > > the
> >> > > > >>> others.
> >> > > > >>> > >> >>> In
> >> > > > >>> > >> >>> > > such a
> >> > > > >>> > >> >>> > > > > policy a user could also check the timestamp
> >> > related
> >> > > > to
> >> > > > >>> the
> >> > > > >>> > >> >>> offset
> >> > > > >>> > >> >>> > for
> >> > > > >>> > >> >>> > > > > allowing or not deletion on time base.
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > > > Paolo Patierno
> >> > > > >>> > >> >>> > > > > Senior Software Engineer (IoT) @ Red Hat
> >> > > > >>> > >> >>> > > > > Microsoft MVP on Azure & IoT
> >> > > > >>> > >> >>> > > > > Microsoft Azure Advisor
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > > > Twitter : @ppatierno<
> >> > > > >>> > >> >>> > >
> >> > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter
> >> > > > >>> .
> >> > > > >>> > >> >>> > > com_ppatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> >> > > > >>> > >> >>> > >
> >> yh7dKgV77XtsUnJ9Rab1gheY&s=43hzTLEDKw2v5Vh0zwkMTaaKD-
> >> > > > >>> > >> >>> > HdJD8d_F4-Bsw25-Y&e=
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > > Linkedin : paolopatierno<
> >> > > > >>> > >> >>> > >
> >> > https://urldefense.proofpoint.com/v2/url?u=http-3A__it.
> >> > > > >>> > >> >>> > >
> >> > > > linkedin.com_in_paolopatierno&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1
> >> > > > >>> > >> ZOg&r=
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> >> > > > >>> > >> >>> > >
> >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Ig0N7Nwf9EHfTJ2pH3jRM1JIdlzXw6
> >> > > > >>> > >> >>> > R5Drocu0TMRLk&e=
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > > Blog : DevExperience<
> >> > > > >>> > >> >>> > >
> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> paolopatierno.wordpress.com_&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=
> >> > > > >>> > >> >>> > >
> >> > > > >>>
> >> EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=h-D-nA7uiy1Z-jta5y-
> >> > > > >>> > >> >>> > >
> >> > > yh7dKgV77XtsUnJ9Rab1gheY&s=Tc9NrTtG2GP7-zRjOHkXHfYI0rncO8_
> >> > > > >>> > >> >>> > jKpedna692z4&e=
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > > > ________________________________
> >> > > > >>> > >> >>> > > > > From: ismaelj@gmail.com <is...@gmail.com> on
> >> > > behalf
> >> > > > >>> of
> >> > > > >>> > >> Ismael
> >> > > > >>> > >> >>> > Juma <
> >> > > > >>> > >> >>> > > > > ismael@juma.me.uk>
> >> > > > >>> > >> >>> > > > > Sent: Wednesday, September 27, 2017 10:18 AM
> >> > > > >>> > >> >>> > > > > To: dev@kafka.apache.org
> >> > > > >>> > >> >>> > > > > Subject: Re: [DISCUSS] KIP-201: Rationalising
> >> > Policy
> >> > > > >>> > >> interfaces
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > > > 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 <
> >> > > > >>> > >> ismael@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....@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 <
> >> > > > >>> > >> >>> ECOMAR@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.bentley@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
> >> > > > >>> > >> >>> > > > > >> >
> >> > > > >>> > >> >>> > > > > >>
> >> > > > >>> > >> >>> > > > > >
> >> > > > >>> > >> >>> > > > > >
> >> > > > >>> > >> >>> > > > >
> >> > > > >>> > >> >>> > > >
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> > > 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
> >> > > > >>> > >> >>> > >
> >> > > > >>> > >> >>> >
> >> > > > >>> > >> >>>
> >> > > > >>> > >> >>
> >> > > > >>> > >> >>
> >> > > > >>> > >> >
> >> > > > >>> > >>
> >> > > > >>> > >
> >> > > > >>> > >
> >> > > > >>>
> >> > > > >>
> >> > > >
> >> > >
> >> >
> >>
> >