Hi Jun,
Ok sounds good.
Justine
On Fri, Apr 12, 2024 at 10:17 AM Jun Rao wrote:
> Hi, Justine,
>
> unstable.metadata.versions.enable is an internal configuration. So, we
> could probably just remove it instead of depreciation. Also, it would be
> useful to make it clear that
Hi, Justine,
unstable.metadata.versions.enable is an internal configuration. So, we
could probably just remove it instead of depreciation. Also, it would be
useful to make it clear that unstable.feature.versions.enable is an
internal configuration.
Thanks,
Jun
On Thu, Apr 11, 2024 at 11:16 AM
Updated. :)
Thanks for the reviews
Justine
On Thu, Apr 11, 2024 at 11:01 AM Jun Rao wrote:
> Hi, Justine,
>
> Thanks for the updated KIP.
>
> Perhaps it's better to name the new config unstable.feature.versions.enable
> since there could be multiple unstable versions.
>
> Other than that, the
Hi, Justine,
Thanks for the updated KIP.
Perhaps it's better to name the new config unstable.feature.versions.enable
since there could be multiple unstable versions.
Other than that, the KIP looks good to me.
Jun
On Thu, Apr 11, 2024 at 9:06 AM Justine Olshan
wrote:
> The original config
The original config was never actually approved in any KIP. But we can say
it is deprecated.
I can change the config name.
Justine
On Thu, Apr 11, 2024 at 8:52 AM Jun Rao wrote:
> Hi, Justine,
>
> Thanks for the updated KIP.
>
> Would unstable.feature.version.enable be a clearer name? Also,
Hi, Justine,
Thanks for the updated KIP.
Would unstable.feature.version.enable be a clearer name? Also, should we
remove/deprecate unstable.metadata.versions.enable in this KIP?
Jun
On Tue, Apr 9, 2024 at 9:09 AM Justine Olshan
wrote:
> Hi Jun,
>
> Makes sense to me. It seems like KIP-1014
Hi Justine,
On Tue, Apr 9, 2024 at 4:19 PM Justine Olshan
wrote:
> As for the validation criteria. It seems like one bit of code that
> validates whether a version is allowed is in the method
> `reasonNotSupported` which checks the range of features available for the
> given feature.
> For
I took a quick look at the code -- looks like the previous behavior was not
to set a top level error if one particular feature had an issue. We can do
that.
I think it could make some sense to specify errors on features that were
not valid and use the top level error to indicate that the request
José,
INVALID_UPDATE_VERSION was added as part of KIP-497. The KIP seems to be
lacking some details on the error.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
https://github.com/apache/kafka/commit/57de67db22eb373f92ec5dd449d317ed2bc8b8d1
The
Hi Justine,
Thanks for the KIP. I see that the KIP doesn't make any updates to the
UpdateFeatures RPC. I was trying to understand how errors will be
communicated to the client.
Are you planning to use the INVALID_UPDATE_VERSION error and overwrite
the ErrorMessage field for all of the
Hi Jun,
Makes sense to me. It seems like KIP-1014 has been inactive recently. I can
update my KIP and mention this change on that discussion thread.
Justine
On Tue, Apr 9, 2024 at 9:01 AM Jun Rao wrote:
> Hi, Justine,
>
> A single config makes sense to me too. We just need to reach consensus
Hi, Justine,
A single config makes sense to me too. We just need to reach consensus with
KIP-1014.
Thanks,
Jun
On Mon, Apr 8, 2024 at 5:06 PM Justine Olshan
wrote:
> Hey Jun,
>
> That's a good question. I think maybe for simplicity, we can have a single
> config?
> If that makes sense, I
Hey Jun,
That's a good question. I think maybe for simplicity, we can have a single
config?
If that makes sense, I will update the KIP.
Justine
On Mon, Apr 8, 2024 at 3:20 PM Jun Rao wrote:
> Hi, Justine,
>
> Thanks for the updated KIP.
>
> One more question related to KIP-1014. It introduced
Hi, Justine,
Thanks for the updated KIP.
One more question related to KIP-1014. It introduced a new
config unstable.metadata.versions.enable. Does each new feature need to
have a corresponding config to enable the testing of unstable features or
should we have a generic config enabling the
I'm hoping this covers the majority of comments. I will go ahead and open
the vote in the next day or so.
Thanks,
Justine
On Wed, Apr 3, 2024 at 3:31 PM Justine Olshan wrote:
> Find and replace has failed me :(
>
> Group version seems a little vague, but we can update it. Hopefully find
> and
Find and replace has failed me :(
Group version seems a little vague, but we can update it. Hopefully find
and replace won't fail me again, otherwise I will get another email on this.
Justine
On Wed, Apr 3, 2024 at 12:15 PM David Jacot
wrote:
> Thanks, Justine.
>
> * Should we also use
Thanks, Justine.
* Should we also use `group.version` (GV) as I suggested in my previous
message in order to be consistent?
* Should we add both names to the `Public Interfaces` section?
* There is still at least one usage of `transaction.protocol.verison` in
the KIP too.
Best,
David
On Wed,
I had missed the David's message yesterday about the naming for transaction
version vs transaction protocol version.
After some offline discussion with Jun, Artem, and David, we agreed that
transaction version is simpler and conveys more than just protocol changes
(flexible records for example)
Updated!
Justine
On Tue, Apr 2, 2024 at 2:40 PM Jun Rao wrote:
> Hi, Justine,
>
> Thanks for the reply.
>
> 21. Sounds good. It would be useful to document that.
>
> 22. Should we add the IV in "metadata.version=17 has no dependencies" too?
>
> Jun
>
>
> On Tue, Apr 2, 2024 at 11:31 AM Justine
Hi, Justine,
Thanks for the reply.
21. Sounds good. It would be useful to document that.
22. Should we add the IV in "metadata.version=17 has no dependencies" too?
Jun
On Tue, Apr 2, 2024 at 11:31 AM Justine Olshan
wrote:
> Jun,
>
> 21. Next producer ID field doesn't need to be populated
Hi José
I originally had each on a new line and then switched to a single line
since it looked like a lot of space. I can switch it back.
I don't think it makes a big difference if we implement it for both tools.
Both tools will have use for it.
I can change the name to feature-dependencies if
Hi Justine,
See my comments below.
On Mon, Apr 1, 2024 at 4:43 PM Justine Olshan
wrote:
> 20. I can update the KIP.
I took a look at the latest KIP.
* Should the output of this command "bin/kafka-features.sh
version-mapping --release-version 3.6-IV1" be something like this:
Jun,
21. Next producer ID field doesn't need to be populated for TV 1. We don't
have the same need to retain this since it is written directly to the
transaction log in InitProducerId. It is only needed for KIP-890 part 2 /
TV 2.
22. We can do that.
Justine
On Tue, Apr 2, 2024 at 10:41 AM Jun
Hi, Justine,
Thanks for the reply.
21. What about the new NextProducerId field? Will that be populated with TV
1?
22. In the dependencies output, should we show both IV and level for
metadata.version too?
Jun
On Mon, Apr 1, 2024 at 4:43 PM Justine Olshan
wrote:
> Hi Jun,
>
> 20. I can
Hi Justine,
Thanks for the KIP. This will be very helpful!
I do have one question regarding the naming of the new flags which is not
totally clear in the KIP. It would be great if we could call them out in
the Public Interfaces section.
My understanding is that the KIP proposes to use
Hi Jun,
> Historically, the format of all records were controlled by MV. Now,
records
in _offset_commit will be controlled by `group.coordinator.version`, is
that right? It would be useful to document that.
Yes. This is correct. The idea is to replace the MV with this new flag. It
will have the
Hi Jun,
20. I can update the KIP.
21. This is used to complete some of the work with KIP-360. (We use
previous producer ID there, but never persisted it which was in the KIP
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89068820)
The KIP also mentions including previous epoch
Hi, Justine,
Thanks for the updated KIP. A couple of more comments.
20. Could we show the output of version-mapping?
21. "Transaction version 1 will include the flexible fields in the
transaction state log, and transaction version 2 will include the changes
to the transactional protocol as
I have also updated the KIP to mention the feature tool's --metadata flag
will be deprecated.
It will still work for users as they learn the new flag, but a warning
indicating the alternatives will be shown.
Justine
On Thu, Mar 28, 2024 at 11:03 AM Justine Olshan
wrote:
> Hi Jun,
>
> For both
Hi Jun,
For both transaction state and group coordinator state, there are only
version 0 records.
KIP-915 introduced flexible versions, but it was never put to use. MV has
never gated these. This KIP will do that. I can include this context in the
KIP.
I'm happy to modify his 1 and 2 to 0 and 1.
Hi, David,
Thanks for the reply.
Historically, the format of all records were controlled by MV. Now, records
in _offset_commit will be controlled by `group.coordinator.version`, is
that right? It would be useful to document that.
Also, we should align on the version numbering. "kafka-feature
Hi David,
I agree that we should use the same mechanism to gate KIP-932 once that
feature reaches production readiness. The precise details of the values will
depend upon the current state of all these flags when that release comes.
Thanks,
Andrew
> On 28 Mar 2024, at 07:11, David Jacot wrote:
Hi, Jun, Justine,
Regarding `group.coordinator.version`, the idea is to use it to gate
records and APIs of the group coordinator. The first use case will be
KIP-848. We will use version 2 of the flag to gate all the new records and
the new ConsumerGroupHeartbeat/Describe APIs present in AK 3.8.
Hi, Justine,
Thanks for the reply.
So, "dependencies" and "version-mapping" will be added to both
kafka-feature and kafka-storage? Could we document that in the tool format
section?
Jun
On Wed, Mar 27, 2024 at 4:01 PM Justine Olshan
wrote:
> Ok. I can remove the info from the describe
Ok. I can remove the info from the describe output.
Dependencies is needed for the storage tool because we want to make sure
the desired versions we are setting will be valid. Version mapping should
be for both tools since we have --release-version for both tools.
I was considering changing the
Hi, Justine,
Thanks for the reply.
Having "kafka-feature dependencies" seems enough to me. We don't need to
include the dependencies in the output of "kafka-feature describe".
We only support "dependencies" in kafka-feature, not kafka-storage. We
probably should do the same for
I had already included this example
bin/kafka-features.sh downgrade --feature metadata.version=16
--transaction.protocol.version=2 // Throws error if metadata version is <
16, and this would be an upgrade
But I have updated the KIP to explicitly say the text you mentioned.
Justine
On Wed, Mar
Hi Justine,
See my comment below.
On Wed, Mar 27, 2024 at 1:31 PM Justine Olshan
wrote:
> The feature command includes the upgrade or downgrade command along with
> the --release-version flag. If some features are not moving in the
> direction mentioned (upgrade or downgrade) the command will
Hey Jun,
Right, sorry this would be one row in an output of all the various features
(transaction.protocol.version, group.coordinator.version) currently set on
the cluster.
If we want to know the dependencies for each of a given feature (ie
transaction.protocol.versions 0, 1, 2, 3 etc) we can
Hi, Justine,
It seems that we need to specify the dependencies for each feature version?
Thanks,
Jun
On Wed, Mar 27, 2024 at 11:28 AM Justine Olshan
wrote:
> Hey Jun,
>
> So just including the dependencies for the currently set features? Along
> with the supported min, max, and finalized
Hey Jun,
So just including the dependencies for the currently set features? Along
with the supported min, max, and finalized versions?
Feature: transaction.protocol.version SupportedMinVersion: 0
SupportedMaxVersion: 2 FinalizedVersionLevel: 1 Epoch: 3 Dependencies:
metadata.version=4
On Wed,
Hi, Justine,
Yes, something like that. We could also extend "kafka-feature describe" by
adding the dependency to every feature in the output.
Thanks,
Jun
On Wed, Mar 27, 2024 at 10:39 AM Justine Olshan
wrote:
> Hi Jun,
>
> We could expose them in a tool. I'm wondering, are you thinking
Hi Jun,
We could expose them in a tool. I'm wondering, are you thinking something
like a command that lists the dependencies for a given feature + version?
Something like:
kafka-feature dependencies --feature transaction.protocol.version=2
> transaction.protocol.verison=2 requires
Hi, Colin,
Thanks for the comments. It's fine if we want to keep the --metadata flag
if it's useful. Then we should add the same flag for kafka-storage for
consistency, right?
Hi, Justine,
Thanks for the reply.
How will a user know about the dependencies among features? Should we
expose them
Hi all,
Thanks for the discussion. Let me respond to a few questions I saw in the
thread (hope I didn't miss any!)
Feature level 0 is "special" because it effectively means that the feature
hasn't been set up. So I could create a foo feature, a bar feature, and a baz
feature
Hi all,
I've added the notes about requiring 3.3-IV0 and that the tool/rpc will
fail if the metadata version is too low.
I will wait for Colin's response on the deprecation. I am not opposed to
deprecating it but want everyone to agree.
Justine
On Tue, Mar 26, 2024 at 12:26 PM José Armando
Hi Justine,
On Mon, Mar 25, 2024 at 5:09 PM Justine Olshan
wrote:
> The reason it is not removed is purely for backwards
> compatibility. Colin had strong feelings about not removing any flags.
We are not saying that we should remove that flag. That would break
backward compatibility of 3.8
Hi, Justine,
Thanks for the reply.
10. If we are exposing group.coordinator.version in this KIP, I think we
need to document what RPCs/records it controls or if it has dependencies on
MV.
15. "For transaction version specifically, I have no metadata version
dependencies besides requiring 3.3
Hi Jun,
I apologize for typos. I thought I got rid of all the non protocol
versions. It is protocol version as per my previous email. I will fix the
KIP.
The group coordinator version is used to upgrade to the new group
coordinator protocol. (KIP-848)
I don't have all the context there.
I would
Hi, Justine,
Thanks for the updated KIP. A few more comments.
10. Could we describe what RPCs group.coordinator.version controls?
12. --metadata METADATA is not removed from kafka-features. Do we have a
justification for this? If so, should we add that to kafka-storage to be
consistent?
14.
I've updated the KIP to include this CLI. For now, I've moved the new api
to the server as a rejected alternative.
It seems like we can keep the mapping in the tool. Given that we also need
to do the same validation for using multiple --feature flags (the request
will look the same to the
Hi Jose,
Sorry for the typos. I think you figured out what I meant.
I can make a new API. There is a risk of creating a ton of very similar
APIs though. Even the ApiVersions api is confusing with its supported and
finalized features fields. I wonder if there is a middle ground here.
I can have
Hi Justine,
Thanks for the update. See my comments below.
On Mon, Mar 25, 2024 at 2:51 PM Justine Olshan
wrote:
> I've updated the KIP with the changes I mentioned earlier. I have not yet
> removed the --feature-version flag from the upgrade tool.
What's the "--feature-version" flag? This is
Hey all,
I've updated the KIP with the changes I mentioned earlier. I have not yet
removed the --feature-version flag from the upgrade tool.
Please take a look at the API to get the versions for a given
metadata version. Right now I'm using ApiVersions request and specifying a
metadata version.
Hey folks,
I didn't have a strong preference for requesting the versions via the tool
only or getting it from the server. Colin seemed to suggest it was "for the
best" to request from the server to make the tool lightweight.
I guess the argument against that is having to build and support another
Hi Justine,
Thanks for the update. Some comments below.
On Wed, Mar 13, 2024 at 10:53 AM Justine Olshan
wrote:
> 4. Include an API to list the features for a given metadata version
I am not opposed to designing and implementing this. I am just
wondering if this is strictly required?
Would
Hi Justine,
Thanks for driving this forward. Comments below.
On Wed, Mar 13, 2024, at 10:51, Justine Olshan wrote:
> Hey folks -- let me summarize my understanding
>
> Artem requested we change the name of transaction version (tv) to
> transaction protocol version (tpv). If I do that, I will
Hey folks -- let me summarize my understanding
Artem requested we change the name of transaction version (tv) to
transaction protocol version (tpv). If I do that, I will also probably
change to group coordinator protocol version (gcpv).
Folks were concerned about upgrades/downgrades with
Hi Justine,
> Are you suggesting it should be called "transaction protocol version" or
"TPV"? I don't mind that, but just wanted to clarify if we want to include
protocol or if simply "transaction version" is enough.
My understanding is that "metadata version" is the version of metadata
Hi Colin,
Thanks for bringing up these points. I believe Jose suggested failing the
command if some features upgrade on the downgrade call and vice versa. This
could address some of the concerns you bring up here. I agree we should not
have an rpc to do both.
As for setting metadata only, we do
Hi Jun and Justine,
My understanding is that in the KIP, --metadata does not do the same thing as
--release-version. The --metadata flag sets the metadata version only, and does
not change any other feature. So neither flag replaces the other.
In general, I'm not sure that this functionality
Hi, Justine,
Thanks for the reply.
11. Yes, we can still have the option to set individual features. I was
suggesting that if one uses the tool without either --release-version
or --features, it will just use the latest version of every feature that it
knows. This is what's proposed in the
Thanks José --
Let me answer a few quick things
If you are not going to deprecate or alias --metadata what happens if
the user uses these flags in one command: "kafka-features upgrade
--metadata 3.8 --feature kraft.version=1"
> I would think as the KIP stands now, we would not accept both
Thanks for the reply Justine. See my comments below:
On Thu, Feb 29, 2024 at 3:39 PM Justine Olshan
wrote:
> I wanted to include multiple features in one command, so it seems like
> features is a better name. I discuss more below about why I think we should
> allow setting multiple features at
Hey folks,
Thanks for the discussion. Let me try to cover everyone's comments.
Artem --
I can add the examples you mentioned. As for naming, right now the feature
is called "transaction version" or "TV". Are you suggesting it should be
called "transaction protocol version" or "TPV"? I don't mind
Hi Justine and Jun,
Thanks for the KIP Justine. See my comments below.
On Wed, Feb 28, 2024 at 3:09 PM Jun Rao wrote:
> 13. KIP-853 also extends the tools to support a new feature kraft.version.
> It would be useful to have alignment between that KIP and this one.
I agree. I took a look at
Hi, Justine,
Thanks for the KIP. A few comments below.
10. Currently, MV controls records, inter-broker RPCs and code logic. With
more features, would each of those be controlled by a separate feature or
multiple features. For example, is the new transaction record format
controlled only by MV
Hi Justine,
Thank you for the KIP. I think the KIP is pretty clear and makes sense to
me. Maybe it would be good to give a little more detail on the
implementation of feature mapping and how the tool would validate the
feature combinations. For example, I'd expect that
bin/kafka-storage.sh
Hey Andrew,
Thanks for taking a look.
I previously didn't include 1. We do plan to use these features immediately
for KIP-890 and KIP-848. If we think it is easier to put the discussion in
those KIP discussions we can, but I fear that it will easily get lost given
the size of the KIPs.
I named
Hi Justine,
Thanks for the KIP. This area of Kafka is complicated and making it easier is
good.
When I use the `kafka-features.sh` tool to describe the features on my cluster,
I find that there’s a
single feature called “metadata.version”. I think this KIP does a handful of
things:
1) It
Hello folks,
I'm proposing a KIP that allows for setting and upgrading new features
(other than metadata version) via the kafka storage format and feature
tools. This KIP extends on the feature versioning changes introduced by
KIP-584 by allowing for the features to be set and upgraded.
Please
71 matches
Mail list logo
