Hi Jun,

Thanks for the feedback and suggestions. Please find my response below.

> 100.6 For every new request, the admin needs to control who is allowed to
> issue that request if security is enabled. So, we need to assign the new
> request a ResourceType and possible AclOperations. See
>
https://cwiki.apache.org/confluence/display/KAFKA/KIP-455%3A+Create+an+Administrative+API+for+Replica+Reassignment
> as an example.

(Kowshik): I don't see any reference to the words ResourceType or
AclOperations
in the KIP. Please let me know how I can use the KIP that you linked to
know how to
setup the appropriate ResourceType and/or ClusterOperation?

> 105. If we change delete to disable, it's better to do this consistently
in
> request protocol and admin api as well.

(Kowshik): The API shouldn't be called 'disable' when it is deleting a
feature.
I've just changed the KIP to use 'delete'. I don't have a strong preference.

> 110. The minVersion/maxVersion for features use int64. Currently, our
> release version schema is major.minor.bugfix (e.g. 2.5.0). It's possible
> for new features to be included in minor releases too. Should we make the
> feature versioning match the release versioning?

(Kowshik): The release version can be mapped to a set of feature versions,
and this can be done, for example in the tool (or even external to the
tool).
Can you please clarify what I'm missing?

> 111. "During regular operations, the data in the ZK node can be mutated
> only via a specific admin API served only by the controller." I am
> wondering why can't the controller auto finalize a feature version after
> all brokers are upgraded? For new users who download the latest version to
> build a new cluster, it's inconvenient for them to have to manually enable
> each feature.

(Kowshik): I agree that there is a trade-off here, but it will help
to decide whether the automation can be thought through in the future
in a follow up KIP, or right now in this KIP. We may invest
in automation, but we have to decide whether we should do it
now or later.

For the inconvenience that you mentioned, do you think the problem that you
mentioned can be  overcome by asking for the cluster operator to run a
bootstrap script  when he/she knows that a specific AK release has been
almost completely deployed in a cluster for the first time? Idea is that the
bootstrap script will know how to map a specific AK release to finalized
feature versions, and run the `kafka-features.sh` tool appropriately against
the cluster.

Now, coming back to your automation proposal/question.
I do see the value of automated feature version finalization, but I also
see
that this will open up several questions and some risks, as explained below.
The answers to these depend on the definition of the automation we choose
to build, and how well does it fit into a kafka deployment.
Basically, it can be unsafe for the controller to finalize feature version
upgrades automatically, without learning about the intent of the cluster
operator.
1. We would sometimes want to lock feature versions only when we have
externally verified
the stability of the broker binary.
2. Sometimes only the cluster operator knows that a cluster upgrade is
complete,
and new brokers are highly unlikely to join the cluster.
3. Only the cluster operator knows that the intent is to deploy the same
version
of the new broker release across the entire cluster (i.e. the latest
downloaded version).
4. For downgrades, it appears the controller still needs some external input
(such as the proposed tool) to finalize a feature version downgrade.

If we have automation, that automation can end up failing in some of the
cases
above. Then, we need a way to declare that the cluster is "not ready" if the
controller cannot automatically finalize some basic required feature version
upgrades across the cluster. We need to make the cluster operator aware in
such a scenario (raise an alert or alike).

> 112. DeleteFeaturesResponse: It seems the apiKey should be 49 instead of
48.

(Kowshik): Done.


Cheers,
Kowshik

On Fri, Apr 3, 2020 at 11:24 AM Jun Rao <j...@confluent.io> wrote:

> Hi, Kowshik,
>
> Thanks for the reply. A few more comments below.
>
> 100.6 For every new request, the admin needs to control who is allowed to
> issue that request if security is enabled. So, we need to assign the new
> request a ResourceType and possible AclOperations. See
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-455%3A+Create+an+Administrative+API+for+Replica+Reassignment
> as
> an example.
>
> 105. If we change delete to disable, it's better to do this consistently in
> request protocol and admin api as well.
>
> 110. The minVersion/maxVersion for features use int64. Currently, our
> release version schema is major.minor.bugfix (e.g. 2.5.0). It's possible
> for new features to be included in minor releases too. Should we make the
> feature versioning match the release versioning?
>
> 111. "During regular operations, the data in the ZK node can be mutated
> only via a specific admin API served only by the controller." I am
> wondering why can't the controller auto finalize a feature version after
> all brokers are upgraded? For new users who download the latest version to
> build a new cluster, it's inconvenient for them to have to manually enable
> each feature.
>
> 112. DeleteFeaturesResponse: It seems the apiKey should be 49 instead of
> 48.
>
> Jun
>
>
> On Fri, Apr 3, 2020 at 1:27 AM Kowshik Prakasam <kpraka...@confluent.io>
> wrote:
>
> > Hey Jun,
> >
> > Thanks a lot for the great feedback! Please note that the design
> > has changed a little bit on the KIP, and we now propagate the finalized
> > features metadata only via ZK watches (instead of UpdateMetadataRequest
> > from the controller).
> >
> > Please find below my response to your questions/feedback, with the prefix
> > "(Kowshik):".
> >
> > > 100. UpdateFeaturesRequest/UpdateFeaturesResponse
> > > 100.1 Since this request waits for responses from brokers, should we
> add
> > a
> > > timeout in the request (like createTopicRequest)?
> >
> > (Kowshik): Great point! Done. I have added a timeout field. Note: we no
> > longer
> > wait for responses from brokers, since the design has been changed so
> that
> > the
> > features information is propagated via ZK. Nevertheless, it is right to
> > have a timeout
> > for the request.
> >
> > > 100.2 The response schema is a bit weird. Typically, the response just
> > > shows an error code and an error message, instead of echoing the
> request.
> >
> > (Kowshik): Great point! Yeah, I have modified it to just return an error
> > code and a message.
> > Previously it was not echoing the "request", rather it was returning the
> > latest set of
> > cluster-wide finalized features (after applying the updates). But you are
> > right,
> > the additional info is not required, so I have removed it from the
> response
> > schema.
> >
> > > 100.3 Should we add a separate request to list/describe the existing
> > > features?
> >
> > (Kowshik): This is already present in the KIP via the 'DescribeFeatures'
> > Admin API,
> > which, underneath covers uses the ApiVersionsRequest to list/describe the
> > existing features. Please read the 'Tooling support' section.
> >
> > > 100.4 We are mixing ADD_OR_UPDATE and DELETE in a single request. For
> > > DELETE, the version field doesn't make sense. So, I guess the broker
> just
> > > ignores this? An alternative way is to have a separate
> > DeleteFeaturesRequest
> >
> > (Kowshik): Great point! I have modified the KIP now to have 2 separate
> > controller APIs
> > serving these different purposes:
> > 1. updateFeatures
> > 2. deleteFeatures
> >
> > > 100.5 In UpdateFeaturesResponse, we have "The monotonically increasing
> > > version of the metadata for finalized features." I am wondering why the
> > > ordering is important?
> >
> > (Kowshik): In the latest KIP write-up, it is called epoch (instead of
> > version), and
> > it is just the ZK node version. Basically, this is the epoch for the
> > cluster-wide
> > finalized feature version metadata. This metadata is served to clients
> via
> > the
> > ApiVersionsResponse (for reads). We propagate updates from the
> '/features'
> > ZK node
> > to all brokers, via ZK watches setup by each broker on the '/features'
> > node.
> >
> > Now here is why the ordering is important:
> > ZK watches don't propagate at the same time. As a result, the
> > ApiVersionsResponse
> > is eventually consistent across brokers. This can introduce cases
> > where clients see an older lower epoch of the features metadata, after a
> > more recent
> > higher epoch was returned at a previous point in time. We expect clients
> > to always employ the rule that the latest received higher epoch of
> metadata
> > always trumps an older smaller epoch. Those clients that are external to
> > Kafka should strongly consider discovering the latest metadata once
> during
> > startup from the brokers, and if required refresh the metadata
> periodically
> > (to get the latest metadata).
> >
> > > 100.6 Could you specify the required ACL for this new request?
> >
> > (Kowshik): What is ACL, and how could I find out which one to specify?
> > Please could you provide me some pointers? I'll be glad to update the
> > KIP once I know the next steps.
> >
> > > 101. For the broker registration ZK node, should we bump up the version
> > in
> > the json?
> >
> > (Kowshik): Great point! Done. I've increased the version in the broker
> json
> > by 1.
> >
> > > 102. For the /features ZK node, not sure if we need the epoch field.
> Each
> > > ZK node has an internal version field that is incremented on every
> > update.
> >
> > (Kowshik): Great point! Done. I'm using the ZK node version now, instead
> of
> > explicitly
> > incremented epoch.
> >
> > > 103. "Enabling the actual semantics of a feature version cluster-wide
> is
> > > left to the discretion of the logic implementing the feature (ex: can
> be
> > > done via dynamic broker config)." Does that mean the broker
> registration
> > ZK
> > > node will be updated dynamically when this happens?
> >
> > (Kowshik): Not really. The text was just conveying that a broker could
> > "know" of
> > a new feature version, but it does not mean the broker should have also
> > activated the effects of the feature version. Knowing vs activation are 2
> > separate things,
> > and the latter can be achieved by dynamic config. I have reworded the
> text
> > to
> > make this clear to the reader.
> >
> >
> > > 104. UpdateMetadataRequest
> > > 104.1 It would be useful to describe when the feature metadata is
> > included
> > > in the request. My understanding is that it's only included if (1)
> there
> > is
> > > a change to the finalized feature; (2) broker restart; (3) controller
> > > failover.
> > > 104.2 The new fields have the following versions. Why are the versions
> 3+
> > > when the top version is bumped to 6?
> > >       "fields":  [
> > >         {"name": "Name", "type":  "string", "versions":  "3+",
> > >           "about": "The name of the feature."},
> > >         {"name":  "Version", "type":  "int64", "versions":  "3+",
> > >           "about": "The finalized version for the feature."}
> > >       ]
> >
> > (Kowshik): With the new improved design, we have completely eliminated
> the
> > need to
> > use UpdateMetadataRequest. This is because we now rely on ZK to deliver
> the
> > notifications for changes to the '/features' ZK node.
> >
> > > 105. kafka-features.sh: Instead of using update/delete, perhaps it's
> > better
> > > to use enable/disable?
> >
> > (Kowshik): For delete, yes, I have changed it so that we instead call it
> > 'disable'.
> > However for 'update', it can now also refer to either an upgrade or a
> > forced downgrade.
> > Therefore, I have left it the way it is, just calling it as just
> 'update'.
> >
> >
> > Cheers,
> > Kowshik
> >
> > On Tue, Mar 31, 2020 at 6:51 PM Jun Rao <j...@confluent.io> wrote:
> >
> > > Hi, Kowshik,
> > >
> > > Thanks for the KIP. Looks good overall. A few comments below.
> > >
> > > 100. UpdateFeaturesRequest/UpdateFeaturesResponse
> > > 100.1 Since this request waits for responses from brokers, should we
> add
> > a
> > > timeout in the request (like createTopicRequest)?
> > > 100.2 The response schema is a bit weird. Typically, the response just
> > > shows an error code and an error message, instead of echoing the
> request.
> > > 100.3 Should we add a separate request to list/describe the existing
> > > features?
> > > 100.4 We are mixing ADD_OR_UPDATE and DELETE in a single request. For
> > > DELETE, the version field doesn't make sense. So, I guess the broker
> just
> > > ignores this? An alternative way is to have a separate
> > > DeleteFeaturesRequest
> > > 100.5 In UpdateFeaturesResponse, we have "The monotonically increasing
> > > version of the metadata for finalized features." I am wondering why the
> > > ordering is important?
> > > 100.6 Could you specify the required ACL for this new request?
> > >
> > > 101. For the broker registration ZK node, should we bump up the version
> > in
> > > the json?
> > >
> > > 102. For the /features ZK node, not sure if we need the epoch field.
> Each
> > > ZK node has an internal version field that is incremented on every
> > update.
> > >
> > > 103. "Enabling the actual semantics of a feature version cluster-wide
> is
> > > left to the discretion of the logic implementing the feature (ex: can
> be
> > > done via dynamic broker config)." Does that mean the broker
> registration
> > ZK
> > > node will be updated dynamically when this happens?
> > >
> > > 104. UpdateMetadataRequest
> > > 104.1 It would be useful to describe when the feature metadata is
> > included
> > > in the request. My understanding is that it's only included if (1)
> there
> > is
> > > a change to the finalized feature; (2) broker restart; (3) controller
> > > failover.
> > > 104.2 The new fields have the following versions. Why are the versions
> 3+
> > > when the top version is bumped to 6?
> > >       "fields":  [
> > >         {"name": "Name", "type":  "string", "versions":  "3+",
> > >           "about": "The name of the feature."},
> > >         {"name":  "Version", "type":  "int64", "versions":  "3+",
> > >           "about": "The finalized version for the feature."}
> > >       ]
> > >
> > > 105. kafka-features.sh: Instead of using update/delete, perhaps it's
> > better
> > > to use enable/disable?
> > >
> > > Jun
> > >
> > > On Tue, Mar 31, 2020 at 5:29 PM Kowshik Prakasam <
> kpraka...@confluent.io
> > >
> > > wrote:
> > >
> > > > Hey Boyang,
> > > >
> > > > Thanks for the great feedback! I have updated the KIP based on your
> > > > feedback.
> > > > Please find my response below for your comments, look for sentences
> > > > starting
> > > > with "(Kowshik)" below.
> > > >
> > > >
> > > > > 1. "When is it safe for the brokers to begin handling EOS traffic"
> > > could
> > > > be
> > > > > converted as "When is it safe for the brokers to start serving new
> > > > > Exactly-Once(EOS) semantics" since EOS is not explained earlier in
> > the
> > > > > context.
> > > >
> > > > (Kowshik): Great point! Done.
> > > >
> > > > > 2. In the *Explanation *section, the metadata version number part
> > > seems a
> > > > > bit blurred. Could you point a reference to later section that we
> > going
> > > > to
> > > > > store it in Zookeeper and update it every time when there is a
> > feature
> > > > > change?
> > > >
> > > > (Kowshik): Great point! Done. I've added a reference in the KIP.
> > > >
> > > >
> > > > > 3. For the feature downgrade, although it's a Non-goal of the KIP,
> > for
> > > > > features such as group coordinator semantics, there is no legal
> > > scenario
> > > > to
> > > > > perform a downgrade at all. So having downgrade door open is pretty
> > > > > error-prone as human faults happen all the time. I'm assuming as
> new
> > > > > features are implemented, it's not very hard to add a flag during
> > > feature
> > > > > creation to indicate whether this feature is "downgradable". Could
> > you
> > > > > explain a bit more on the extra engineering effort for shipping
> this
> > > KIP
> > > > > with downgrade protection in place?
> > > >
> > > > (Kowshik): Great point! I'd agree and disagree here. While I agree
> that
> > > > accidental
> > > > downgrades can cause problems, I also think sometimes downgrades
> should
> > > > be allowed for emergency reasons (not all downgrades cause issues).
> > > > It is just subjective to the feature being downgraded.
> > > >
> > > > To be more strict about feature version downgrades, I have modified
> the
> > > KIP
> > > > proposing that we mandate a `--force-downgrade` flag be used in the
> > > > UPDATE_FEATURES api
> > > > and the tooling, whenever the human is downgrading a finalized
> feature
> > > > version.
> > > > Hopefully this should cover the requirement, until we find the need
> for
> > > > advanced downgrade support.
> > > >
> > > > > 4. "Each broker’s supported dictionary of feature versions will be
> > > > defined
> > > > > in the broker code." So this means in order to restrict a certain
> > > > feature,
> > > > > we need to start the broker first and then send a feature gating
> > > request
> > > > > immediately, which introduces a time gap and the intended-to-close
> > > > feature
> > > > > could actually serve request during this phase. Do you think we
> > should
> > > > also
> > > > > support configurations as well so that admin user could freely roll
> > up
> > > a
> > > > > cluster with all nodes complying the same feature gating, without
> > > > worrying
> > > > > about the turnaround time to propagate the message only after the
> > > cluster
> > > > > starts up?
> > > >
> > > > (Kowshik): This is a great point/question. One of the expectations
> out
> > of
> > > > this KIP, which is
> > > > already followed in the broker, is the following.
> > > >  - Imagine at time T1 the broker starts up and registers it’s
> presence
> > in
> > > > ZK,
> > > >    along with advertising it’s supported features.
> > > >  - Imagine at a future time T2 the broker receives the
> > > > UpdateMetadataRequest
> > > >    from the controller, which contains the latest finalized features
> as
> > > > seen by
> > > >    the controller. The broker validates this data against it’s
> > supported
> > > > features to
> > > >    make sure there is no mismatch (it will shutdown if there is an
> > > > incompatibility).
> > > >
> > > > It is expected that during the time between the 2 events T1 and T2,
> the
> > > > broker is
> > > > almost a silent entity in the cluster. It does not add any value to
> the
> > > > cluster, or carry
> > > > out any important broker activities. By “important”, I mean it is not
> > > doing
> > > > mutations
> > > > on it’s persistence, not mutating critical in-memory state, won’t be
> > > > serving
> > > > produce/fetch requests. Note it doesn’t even know it’s assigned
> > > partitions
> > > > until
> > > > it receives UpdateMetadataRequest from controller. Anything the
> broker
> > is
> > > > doing up
> > > > until this point is not damaging/useful.
> > > >
> > > > I’ve clarified the above in the KIP, see this new section:
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+scheme+for+features#KIP-584:Versioningschemeforfeatures-Incompatiblebrokerlifetime
> > > > .
> > > >
> > > > > 5. "adding a new Feature, updating or deleting an existing
> Feature",
> > > may
> > > > be
> > > > > I misunderstood something, I thought the features are defined in
> > broker
> > > > > code, so admin could not really create a new feature?
> > > >
> > > > (Kowshik): Great point! You understood this right. Here adding a
> > feature
> > > > means we are
> > > > adding a cluster-wide finalized *max* version for a feature that was
> > > > previously never finalized.
> > > > I have clarified this in the KIP now.
> > > >
> > > > > 6. I think we need a separate error code like
> > > FEATURE_UPDATE_IN_PROGRESS
> > > > to
> > > > > reject a concurrent feature update request.
> > > >
> > > > (Kowshik): Great point! I have modified the KIP adding the above (see
> > > > 'Tooling support -> Admin API changes').
> > > >
> > > > > 7. I think we haven't discussed the alternative solution to pass
> the
> > > > > feature information through Zookeeper. Is that mentioned in the KIP
> > to
> > > > > justify why using UpdateMetadata is more favorable?
> > > >
> > > > (Kowshik): Nice question! The broker reads finalized feature info
> > stored
> > > in
> > > > ZK,
> > > > only during startup when it does a validation. When serving
> > > > `ApiVersionsRequest`, the
> > > > broker does not read this info from ZK directly. I'd imagine the risk
> > is
> > > > that it can increase
> > > > the ZK read QPS which can be a bottleneck for the system. Today, in
> > Kafka
> > > > we use the
> > > > controller to fan out ZK updates to brokers and we want to stick to
> > that
> > > > pattern to avoid
> > > > the ZK read bottleneck when serving `ApiVersionsRequest`.
> > > >
> > > > > 8. I was under the impression that user could configure a range of
> > > > > supported versions, what's the trade-off for allowing single
> > finalized
> > > > > version only?
> > > >
> > > > (Kowshik): Great question! The finalized version of a feature
> basically
> > > > refers to
> > > > the cluster-wide finalized feature "maximum" version. For example, if
> > the
> > > > 'group_coordinator' feature
> > > > has the finalized version set to 10, then, it means that cluster-wide
> > all
> > > > versions upto v10 are
> > > > supported for this feature. However, note that if some version (ex:
> v0)
> > > > gets deprecated
> > > > for this feature, then we don’t convey that using this scheme (also
> > > > supporting deprecation is a non-goal).
> > > >
> > > > (Kowshik): I’ve now modified the KIP at all points, refering to
> > finalized
> > > > feature "maximum" versions.
> > > >
> > > > > 9. One minor syntax fix: Note that here the "client" here may be a
> > > > producer
> > > >
> > > > (Kowshik): Great point! Done.
> > > >
> > > >
> > > > Cheers,
> > > > Kowshik
> > > >
> > > >
> > > > On Tue, Mar 31, 2020 at 1:17 PM Boyang Chen <
> > reluctanthero...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hey Kowshik,
> > > > >
> > > > > thanks for the revised KIP. Got a couple of questions:
> > > > >
> > > > > 1. "When is it safe for the brokers to begin handling EOS traffic"
> > > could
> > > > be
> > > > > converted as "When is it safe for the brokers to start serving new
> > > > > Exactly-Once(EOS) semantics" since EOS is not explained earlier in
> > the
> > > > > context.
> > > > >
> > > > > 2. In the *Explanation *section, the metadata version number part
> > > seems a
> > > > > bit blurred. Could you point a reference to later section that we
> > going
> > > > to
> > > > > store it in Zookeeper and update it every time when there is a
> > feature
> > > > > change?
> > > > >
> > > > > 3. For the feature downgrade, although it's a Non-goal of the KIP,
> > for
> > > > > features such as group coordinator semantics, there is no legal
> > > scenario
> > > > to
> > > > > perform a downgrade at all. So having downgrade door open is pretty
> > > > > error-prone as human faults happen all the time. I'm assuming as
> new
> > > > > features are implemented, it's not very hard to add a flag during
> > > feature
> > > > > creation to indicate whether this feature is "downgradable". Could
> > you
> > > > > explain a bit more on the extra engineering effort for shipping
> this
> > > KIP
> > > > > with downgrade protection in place?
> > > > >
> > > > > 4. "Each broker’s supported dictionary of feature versions will be
> > > > defined
> > > > > in the broker code." So this means in order to restrict a certain
> > > > feature,
> > > > > we need to start the broker first and then send a feature gating
> > > request
> > > > > immediately, which introduces a time gap and the intended-to-close
> > > > feature
> > > > > could actually serve request during this phase. Do you think we
> > should
> > > > also
> > > > > support configurations as well so that admin user could freely roll
> > up
> > > a
> > > > > cluster with all nodes complying the same feature gating, without
> > > > worrying
> > > > > about the turnaround time to propagate the message only after the
> > > cluster
> > > > > starts up?
> > > > >
> > > > > 5. "adding a new Feature, updating or deleting an existing
> Feature",
> > > may
> > > > be
> > > > > I misunderstood something, I thought the features are defined in
> > broker
> > > > > code, so admin could not really create a new feature?
> > > > >
> > > > > 6. I think we need a separate error code like
> > > FEATURE_UPDATE_IN_PROGRESS
> > > > to
> > > > > reject a concurrent feature update request.
> > > > >
> > > > > 7. I think we haven't discussed the alternative solution to pass
> the
> > > > > feature information through Zookeeper. Is that mentioned in the KIP
> > to
> > > > > justify why using UpdateMetadata is more favorable?
> > > > >
> > > > > 8. I was under the impression that user could configure a range of
> > > > > supported versions, what's the trade-off for allowing single
> > finalized
> > > > > version only?
> > > > >
> > > > > 9. One minor syntax fix: Note that here the "client" here may be a
> > > > producer
> > > > >
> > > > > Boyang
> > > > >
> > > > > On Mon, Mar 30, 2020 at 4:53 PM Colin McCabe <cmcc...@apache.org>
> > > wrote:
> > > > >
> > > > > > On Thu, Mar 26, 2020, at 19:24, Kowshik Prakasam wrote:
> > > > > > > Hi Colin,
> > > > > > >
> > > > > > > Thanks for the feedback! I've changed the KIP to address your
> > > > > > > suggestions.
> > > > > > > Please find below my explanation. Here is a link to KIP 584:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+scheme+for+features
> > > > > > > .
> > > > > > >
> > > > > > > 1. '__data_version__' is the version of the finalized feature
> > > > metadata
> > > > > > > (i.e. actual ZK node contents), while the '__schema_version__'
> is
> > > the
> > > > > > > version of the schema of the data persisted in ZK. These serve
> > > > > different
> > > > > > > purposes. '__data_version__' is is useful mainly to clients
> > during
> > > > > reads,
> > > > > > > to differentiate between the 2 versions of eventually
> consistent
> > > > > > 'finalized
> > > > > > > features' metadata (i.e. larger metadata version is more
> recent).
> > > > > > > '__schema_version__' provides an additional degree of
> > flexibility,
> > > > > where
> > > > > > if
> > > > > > > we decide to change the schema for '/features' node in ZK (in
> the
> > > > > > future),
> > > > > > > then we can manage broker roll outs suitably (i.e.
> > > > > > > serialization/deserialization of the ZK data can be handled
> > > safely).
> > > > > >
> > > > > > Hi Kowshik,
> > > > > >
> > > > > > If you're talking about a number that lets you know if data is
> more
> > > or
> > > > > > less recent, we would typically call that an epoch, and not a
> > > version.
> > > > > For
> > > > > > the ZK data structures, the word "version" is typically reserved
> > for
> > > > > > describing changes to the overall schema of the data that is
> > written
> > > to
> > > > > > ZooKeeper.  We don't even really change the "version" of those
> > > schemas
> > > > > that
> > > > > > much, since most changes are backwards-compatible.  But we do
> > include
> > > > > that
> > > > > > version field just in case.
> > > > > >
> > > > > > I don't think we really need an epoch here, though, since we can
> > just
> > > > > look
> > > > > > at the broker epoch.  Whenever the broker registers, its epoch
> will
> > > be
> > > > > > greater than the previous broker epoch.  And the newly registered
> > > data
> > > > > will
> > > > > > take priority.  This will be a lot simpler than adding a separate
> > > epoch
> > > > > > system, I think.
> > > > > >
> > > > > > >
> > > > > > > 2. Regarding admin client needing min and max information - you
> > are
> > > > > > right!
> > > > > > > I've changed the KIP such that the Admin API also allows the
> user
> > > to
> > > > > read
> > > > > > > 'supported features' from a specific broker. Please look at the
> > > > section
> > > > > > > "Admin API changes".
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > > >
> > > > > > > 3. Regarding the use of `long` vs `Long` - it was not
> deliberate.
> > > > I've
> > > > > > > improved the KIP to just use `long` at all places.
> > > > > >
> > > > > > Sounds good.
> > > > > >
> > > > > > >
> > > > > > > 4. Regarding kafka.admin.FeatureCommand tool - you are right!
> > I've
> > > > > > updated
> > > > > > > the KIP sketching the functionality provided by this tool, with
> > > some
> > > > > > > examples. Please look at the section "Tooling support
> examples".
> > > > > > >
> > > > > > > Thank you!
> > > > > >
> > > > > >
> > > > > > Thanks, Kowshik.
> > > > > >
> > > > > > cheers,
> > > > > > Colin
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Kowshik
> > > > > > >
> > > > > > > On Wed, Mar 25, 2020 at 11:31 PM Colin McCabe <
> > cmcc...@apache.org>
> > > > > > wrote:
> > > > > > >
> > > > > > > > Thanks, Kowshik, this looks good.
> > > > > > > >
> > > > > > > > In the "Schema" section, do we really need both
> > > __schema_version__
> > > > > and
> > > > > > > > __data_version__?  Can we just have a single version field
> > here?
> > > > > > > >
> > > > > > > > Shouldn't the Admin(Client) function have some way to get the
> > min
> > > > and
> > > > > > max
> > > > > > > > information that we're exposing as well?  I guess we could
> have
> > > > min,
> > > > > > max,
> > > > > > > > and current.  Unrelated: is the use of Long rather than long
> > > > > deliberate
> > > > > > > > here?
> > > > > > > >
> > > > > > > > It would be good to describe how the command line tool
> > > > > > > > kafka.admin.FeatureCommand will work.  For example the flags
> > that
> > > > it
> > > > > > will
> > > > > > > > take and the output that it will generate to STDOUT.
> > > > > > > >
> > > > > > > > cheers,
> > > > > > > > Colin
> > > > > > > >
> > > > > > > >
> > > > > > > > On Tue, Mar 24, 2020, at 17:08, Kowshik Prakasam wrote:
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > I've opened KIP-584
> > > <https://issues.apache.org/jira/browse/KIP-584> <
> > > > https://issues.apache.org/jira/browse/KIP-584
> > > > > >
> > > > > > > > > which
> > > > > > > > > is intended to provide a versioning scheme for features.
> I'd
> > > like
> > > > > to
> > > > > > use
> > > > > > > > > this thread to discuss the same. I'd appreciate any
> feedback
> > on
> > > > > this.
> > > > > > > > > Here
> > > > > > > > > is a link to KIP-584
> > > <https://issues.apache.org/jira/browse/KIP-584>:
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+scheme+for+features
> > > > > > > > >  .
> > > > > > > > >
> > > > > > > > > Thank you!
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Cheers,
> > > > > > > > > Kowshik
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Reply via email to