Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
cmccabe commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2176610110 I commented in the other PR that I think we should just revert KIP-1005 from 3.8 if it's not finished. https://github.com/apache/kafka/pull/16347#issuecomment-2176593301 We can implement it in 3.9-IV0. What do you think? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
cmccabe commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1644812539
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
There is really no reason to leave unstable MVs unused. I think the issue
with IBP_3_7_IV3 was a misunderstanding. The whole point of an unstable MV is
you can do whatever you want.
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
There is really no reason to leave unstable MVs unused. I think the issue
with IBP_3_7_IV3 was a misunderstanding. The whole point of an unstable MV is
you can do whatever you want (until it becomes stable).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2176575249 @cmccabe and @jlprat : Here is a summary of of the issue that this PR is trying to fix. [KIP-1005](https://cwiki.apache.org/confluence/display/KAFKA/KIP-1005%3A+Expose+EarliestLocalOffset+and+TieredOffset) proposes to (1) add support for latest-tiered timestamp (-5) in ListOffset request (2) expose earliest-local (-4) and latest-tiered timestamp (-5) in AdminClient and GetOffsetShell tool. (1) was implemented in the 3.8 branch, but was implemented incorrectly since it didn't bump up the request version (see https://issues.apache.org/jira/browse/KAFKA-16480). (2) hasn't been implemented yet. > Does this add a new feature to 3.8 or not? Some comments seems to indicate that it does, but there was also a comment that this was just "fixing a mistake" For this PR, we just want to fix (1). > Is the last version of ListOffsets unstable or not? Title of the PR indicates that it should be, but the PR itself doesn't set "latestVersionUnstable": true. in ListOffsetsRequest. The latest PR makes the latest version ListOffsets stable. So, we need to change the description of this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
cmccabe commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2174437063 3.8 is done at this point. These changes can be done in 3.9-IV0 once #16347 is in. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2172995019 The latest test failures, in my opinion, are unrelated to the change -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1642582634
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -21,10 +21,10 @@
public enum TestFeatureVersion implements FeatureVersion {
-// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
+// TEST_1 released right before MV 3.7-IV0 was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
-// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+// TEST_2 released right before MV 4.0-IV0 was released, and it depends on
this metadata version
+TEST_2(2, MetadataVersion.IBP_4_0_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IV0.featureLevel()));
Review Comment:
I put this to 3.8-IV1, but since 3.8-IV1 is now marked as a production
version a test (testLatestFeaturesWithOldMetadataVersion) fails with a message
like
```
test.feature.version could not be set to 2 because it depends on
metadata.version level 21
```
And two tests (testUnstableTestVersion, testUnstableFeatureThrowsError) fail
with
```
Expected java.lang.IllegalArgumentException to be thrown, but nothing was
thrown.
```
As such, I have a preference that we keep this as IBP_4_0_IV0 which appears
to exercise all scenarios successfully.
What is your opinion @jolshan? Do you want a stable version to be exercised
as part of TEST_2 right now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1640456908
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -21,10 +21,10 @@
public enum TestFeatureVersion implements FeatureVersion {
-// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
+// TEST_1 released right before MV 3.7-IV0 was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
-// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+// TEST_2 released right before MV 4.0-IV0 was released, and it depends on
this metadata version
+TEST_2(2, MetadataVersion.IBP_4_0_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IV0.featureLevel()));
Review Comment:
Thanks, Christo. Have you pushed the change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
jolshan commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1638519065
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Heh, this comment thread is becoming the KIP-1014 discussion. I suppose
while we have not completed that discussion we can just pick an approach for
now. I don't feel super strongly about this for the one more MV while we don't
have the conclusive answer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1638285964
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -21,10 +21,10 @@
public enum TestFeatureVersion implements FeatureVersion {
-// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
+// TEST_1 released right before MV 3.7-IV0 was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
-// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+// TEST_2 released right before MV 4.0-IV0 was released, and it depends on
this metadata version
+TEST_2(2, MetadataVersion.IBP_4_0_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IV0.featureLevel()));
Review Comment:
Ah, apologies, I missed Justine's comment, I will make the change now, thank
you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1637247133
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
ELR was originally associated with IBP_3_7_IV3. When it's moved IBP_3_8_IV0,
we left IBP_3_7_IV3 unused since IBP_3_7_IV4 has already been implemented. We
didn't reuse IBP_3_7_IV3. Here, there is no 3.8 IV after IBP_3_8_IV0. So, we
could reuse IBP_3_8_IV0 for ListOffset. However, it seems that a simpler and
more consistent policy is to never reuse an IV?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
jolshan commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1637231197
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
I'm not sure I understand the benefit of leaving it unused. I would suspect
if there are many ongoing features, we will have a lot of unused versions and
that is confusing it its own way. I was hoping the details of KIP-1014 would be
finalized by now so that there was clear guidance for this sort of state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1637226783
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
@jolshan: What's the benefit of reusing 3.8-IV0 for ListOffset instead of
creating a new 3.8-IV1? To me, creating 3.8-IV1 makes it a bit easier to
understand the change history. It leaves an IV unused, but that doesn't seem a
big concern.
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -21,10 +21,10 @@
public enum TestFeatureVersion implements FeatureVersion {
-// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
+// TEST_1 released right before MV 3.7-IV0 was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
-// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+// TEST_2 released right before MV 4.0-IV0 was released, and it depends on
this metadata version
+TEST_2(2, MetadataVersion.IBP_4_0_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IV0.featureLevel()));
Review Comment:
@jolshan left a comment earlier.
https://github.com/apache/kafka/pull/15673#discussion_r1630117332
Since she plans to create a new feature TEST_3 for unstable version. We
probably can keep this at 3_8_IV1 since it's a stable version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
CalvinConfluent commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1635552897
##
metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java:
##
@@ -371,7 +371,7 @@ public void
testPartitionRegistrationToRecord_ElrShouldBeNullIfEmpty() {
setPartitionEpoch(0);
List exceptions = new ArrayList<>();
ImageWriterOptions options = new ImageWriterOptions.Builder().
-setMetadataVersion(MetadataVersion.IBP_3_8_IV0).
+setMetadataVersion(MetadataVersion.IBP_3_8_IV1).
Review Comment:
Yes, it requires IBP_4_0_IV0
##
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##
@@ -394,7 +394,7 @@ public void testUncleanShutdownBroker() throws Throwable {
new BrokerRegistrationRequestData().
setBrokerId(brokerId).
setClusterId(active.clusterId()).
-
setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV0)).
+
setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV1)).
Review Comment:
Yes, it requires IBP_4_0_IV0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
jolshan commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1630407917
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
If ELR is being pushed to 4.0 it may be worth making 4.0-IV1 since both the
group version and the ELR are not stable.
But yes, I do mean that if we plan to release a new MV in 3.8, we should
just use 3.8-IV0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1630381323
##
metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java:
##
@@ -371,7 +371,7 @@ public void
testPartitionRegistrationToRecord_ElrShouldBeNullIfEmpty() {
setPartitionEpoch(0);
List exceptions = new ArrayList<>();
ImageWriterOptions options = new ImageWriterOptions.Builder().
-setMetadataVersion(MetadataVersion.IBP_3_8_IV0).
+setMetadataVersion(MetadataVersion.IBP_3_8_IV1).
Review Comment:
Hmm, this is testing ELR. So it seems that it needs to be IBP_4_0_IV0.
@CalvinConfluent : Could you confirm this?
##
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##
@@ -394,7 +394,7 @@ public void testUncleanShutdownBroker() throws Throwable {
new BrokerRegistrationRequestData().
setBrokerId(brokerId).
setClusterId(active.clusterId()).
-
setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV0)).
+
setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV1)).
Review Comment:
Hmm, this is testing ELR. So it seems that it needs to be IBP_4_0_IV0. Ditto
below. @CalvinConfluent : Could you confirm this?
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -23,8 +23,8 @@ public enum TestFeatureVersion implements FeatureVersion {
// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
-// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+// TEST_2 released right before MV 4.0-IVO was released, and it depends on
this metadata version
Review Comment:
4.0-IVO => 4.0-IV0 This is an existing issue. The trailing number should be
0, not an O.
Ditto for 3.7-IVO above.
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
@jolshan : Just to clarify. Are you saying that ELR can just use MV 4.0 IV0,
instead of creating a new one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
jolshan commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1630120299
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Hey folks, the point of unstable metadata versions is that we should be able
to move stuff around and not have to keep creating new ones.
I was hoping that KIP would be finalized so we could move forward with this
protocol.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
jolshan commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1630117332
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -24,7 +24,7 @@ public enum TestFeatureVersion implements FeatureVersion {
// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+TEST_2(2, MetadataVersion.IBP_4_0_IVO,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IVO.featureLevel()));
Review Comment:
I have a plan for this. I plan to keep the latest version unstable and
create a TEST_3. TEST_2 will be used for the dependency test and it can be
either 3.8 or 4.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r162682
##
tools/src/test/java/org/apache/kafka/tools/FeatureCommandTest.java:
##
@@ -69,10 +69,10 @@ public void testDescribeWithKRaft(ClusterInstance cluster) {
// Change expected message to reflect latest MetadataVersion
(SupportedMaxVersion increases when adding a new version)
assertEquals("Feature: metadata.version\tSupportedMinVersion:
3.0-IV1\t" +
-"SupportedMaxVersion: 4.0-IV0\tFinalizedVersionLevel:
3.3-IV1\t", outputWithoutEpoch(features.get(1)));
+"SupportedMaxVersion: 3.8-IV1\tFinalizedVersionLevel:
3.3-IV1\t", outputWithoutEpoch(features.get(1)));
Review Comment:
Yeah, upon rebasing this does need to be 4.0-IV0, I suspect my workspace was
confused when I ran the test locally. Reverted!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
CalvinConfluent commented on code in PR #15673: URL: https://github.com/apache/kafka/pull/15673#discussion_r1629872319 ## metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java: ## @@ -283,7 +283,7 @@ private static Stream metadataVersionsForTestPartitionRegistration() return Stream.of( MetadataVersion.IBP_3_7_IV1, MetadataVersion.IBP_3_7_IV2, -MetadataVersion.IBP_3_8_IV0 +MetadataVersion.IBP_3_8_IV1 Review Comment: QuorumControllerTest.java: testUncleanShutdownBroker -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
CalvinConfluent commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1629856039
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -201,11 +201,15 @@ public enum MetadataVersion {
// Add new fetch request version for KIP-951
IBP_3_7_IV4(19, "3.7", "IV4", false),
-// Add ELR related supports (KIP-966).
+// Similar to IBP_3_7_IV3, IBP_3_8_IV0 was reserved for ELR support
(KIP-966) but has
+// been moved forward to a later release requiring a new MetadataVersion.
IBP_3_8_IV0(20, "3.8", "IV0", true),
+// Add ListOffsets V9 (KIP-1005)
+IBP_3_8_IV1(21, "3.8", "IV1", false),
+
// Introduce version 1 of the GroupVersion feature (KIP-848).
-IBP_4_0_IV0(21, "4.0", "IV0", false);
+IBP_4_0_IV0(22, "4.0", "IV0", false);
Review Comment:
Yes, sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1628455890
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -24,7 +24,7 @@ public enum TestFeatureVersion implements FeatureVersion {
// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+TEST_2(2, MetadataVersion.IBP_4_0_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IV0.featureLevel()));
Review Comment:
Could we adjust the comment above on "MV 3.8-IVO"?
##
metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java:
##
@@ -283,7 +283,7 @@ private static Stream
metadataVersionsForTestPartitionRegistration()
return Stream.of(
MetadataVersion.IBP_3_7_IV1,
MetadataVersion.IBP_3_7_IV2,
-MetadataVersion.IBP_3_8_IV0
+MetadataVersion.IBP_3_8_IV1
Review Comment:
This corresponds to ELR and needs to be changed to BP_4_0_IV0 like
PartitionChangeBuilderTest.java. Ditto below.
@CalvinConfluent : Could you confirm that? Also, could you check if there is
any other change needed for ELR in this PR?
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -201,11 +201,15 @@ public enum MetadataVersion {
// Add new fetch request version for KIP-951
IBP_3_7_IV4(19, "3.7", "IV4", false),
-// Add ELR related supports (KIP-966).
+// Similar to IBP_3_7_IV3, IBP_3_8_IV0 was reserved for ELR support
(KIP-966) but has
+// been moved forward to a later release requiring a new MetadataVersion.
IBP_3_8_IV0(20, "3.8", "IV0", true),
+// Add ListOffsets V9 (KIP-1005)
+IBP_3_8_IV1(21, "3.8", "IV1", false),
+
// Introduce version 1 of the GroupVersion feature (KIP-848).
-IBP_4_0_IV0(21, "4.0", "IV0", false);
+IBP_4_0_IV0(22, "4.0", "IV0", false);
Review Comment:
Perhaps we could add a comment that IBP_4_0_IV0 also corresponds to ELR for
now. @CalvinConfluent : Is this ok with you for now?
##
tools/src/test/java/org/apache/kafka/tools/FeatureCommandTest.java:
##
@@ -69,10 +69,10 @@ public void testDescribeWithKRaft(ClusterInstance cluster) {
// Change expected message to reflect latest MetadataVersion
(SupportedMaxVersion increases when adding a new version)
assertEquals("Feature: metadata.version\tSupportedMinVersion:
3.0-IV1\t" +
-"SupportedMaxVersion: 4.0-IV0\tFinalizedVersionLevel:
3.3-IV1\t", outputWithoutEpoch(features.get(1)));
+"SupportedMaxVersion: 3.8-IV1\tFinalizedVersionLevel:
3.3-IV1\t", outputWithoutEpoch(features.get(1)));
Review Comment:
The test seems to fail with this change. This test is configured with
unstable.feature.versions.enable=true. So, returning 4.0-IV0 for max supported
version is correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1626433087
##
tools/src/test/java/org/apache/kafka/tools/FeatureCommandTest.java:
##
@@ -69,10 +69,10 @@ public void testDescribeWithKRaft(ClusterInstance cluster) {
// Change expected message to reflect latest MetadataVersion
(SupportedMaxVersion increases when adding a new version)
assertEquals("Feature: metadata.version\tSupportedMinVersion:
3.0-IV1\t" +
-"SupportedMaxVersion: 4.0-IV0\tFinalizedVersionLevel:
3.3-IV1\t", outputWithoutEpoch(features.get(1)));
+"SupportedMaxVersion: 3.8-IV1\tFinalizedVersionLevel:
3.3-IV1\t", outputWithoutEpoch(features.get(1)));
Review Comment:
I haven't had the time to fully check why this had to change from 4.0-IV0 to
3.8-IV1, but I will circle back tomorrow to confirm should you think it is a
problem
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
jolshan commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1626234150
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -24,7 +24,7 @@ public enum TestFeatureVersion implements FeatureVersion {
// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+TEST_2(2, MetadataVersion.IBP_4_0_IVO,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IVO.featureLevel()));
Review Comment:
Yeah, I was thinking about this. If we should always have this version be an
unstable version.
I think there are trade offs either way. We get the unstable version test
coverage, but we have to keep updating it every time the MV becomes stable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1625727679
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Okay, but can the people working on ELR ensure they reserve the new IBP as
part of their work? I want to constrain this PR to just allocate IBP_3_8_IV1
for 1005 to be completed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673: URL: https://github.com/apache/kafka/pull/15673#discussion_r1625722178 ## clients/src/main/resources/common/message/ListOffsetsRequest.json: ## @@ -34,7 +34,9 @@ // Version 7 enables listing offsets by max timestamp (KIP-734). // // Version 8 enables listing offsets by local log start offset (KIP-405). - "validVersions": "0-8", + // + // Version 9 enables listing offsets by last tiered offset (KIP-1005). Review Comment: Yes, correct, but I want to get this PR in before I move to that. I do not want to bunch all of these changes in the same PR -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1624752626
##
server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java:
##
@@ -185,6 +185,7 @@ public void testFromVersionString() {
assertEquals(IBP_3_7_IV4,
MetadataVersion.fromVersionString("3.7-IV4"));
assertEquals(IBP_3_8_IV0,
MetadataVersion.fromVersionString("3.8-IV0"));
+assertEquals(IBP_3_8_IV1,
MetadataVersion.fromVersionString("3.8-IV1"));
Review Comment:
Should we add a comment that IBP_3_8_IV1 is the latest product version of
3.8?
##
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##
@@ -24,7 +24,7 @@ public enum TestFeatureVersion implements FeatureVersion {
// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
-TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+TEST_2(2, MetadataVersion.IBP_4_0_IVO,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IVO.featureLevel()));
Review Comment:
@jolshan : Should we keep the MV for TEST_2 fixed at IBP_3_8_IV0?
##
server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java:
##
@@ -297,6 +298,7 @@ public void testVersion() {
assertEquals("3.7-IV3", IBP_3_7_IV3.version());
assertEquals("3.7-IV4", IBP_3_7_IV4.version());
assertEquals("3.8-IV0", IBP_3_8_IV0.version());
+assertEquals("3.8-IV1", IBP_3_8_IV1.version());
Review Comment:
1. Should we add `IBP_3_8_IV1` in `testShortVersion()` too?
2. Should we change `testIsElrSupported()` to use `IBP_4_0_IVO`?
3. Should we update
`FeatureCommandTest.testDescribeWithKRaftAndBootstrapControllers` from
`metadataVersion = MetadataVersion.IBP_3_7_IV4` to `metadataVersion =
MetadataVersion.IBP_3_8_IV1`?
##
clients/src/main/resources/common/message/ListOffsetsRequest.json:
##
@@ -34,7 +34,9 @@
// Version 7 enables listing offsets by max timestamp (KIP-734).
//
// Version 8 enables listing offsets by local log start offset (KIP-405).
- "validVersions": "0-8",
+ //
+ // Version 9 enables listing offsets by last tiered offset (KIP-1005).
Review Comment:
To let the AdminClient use this, we need to add a new type of OffsetSpec and
a way to set oldestAllowedVersion in ListOffsetsRequest.Build to version 9,
right? See https://github.com/apache/kafka/pull/10760/files
##
metadata/src/test/java/org/apache/kafka/controller/PartitionChangeBuilderTest.java:
##
@@ -125,7 +125,7 @@ private static MetadataVersion
metadataVersionForPartitionChangeRecordVersion(sh
case (short) 1:
return MetadataVersion.IBP_3_7_IV2;
case (short) 2:
-return MetadataVersion.IBP_3_8_IV0;
+return MetadataVersion.IBP_3_8_IV1;
Review Comment:
We need to change it to return `IBP_4_0_IVO` for version 2 of
PartitionChangeRecord since the version 2 change is for ELR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
CalvinConfluent commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1624719519
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
The 4.0 IV0 is taken, I think we should use IV1?
```
// Introduce version 1 of the GroupVersion feature (KIP-848).
IBP_4_0_IVO(21, "4.0", "IV0", false);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1624631817
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Okay, I believe I have moved ELR to at least IBP_4_0_IV0, but I would prefer
to leave the rest of the test changes for when ELR is launched i.e. I have only
changed the tests to reflect the changes needed for 3.8.1
##
core/src/test/scala/unit/kafka/server/ListOffsetsRequestTest.scala:
##
@@ -55,7 +55,7 @@ class ListOffsetsRequestTest extends BaseRequestTest {
.build()
val debugReplicaRequest = ListOffsetsRequest.Builder
- .forReplica(ApiKeys.LIST_OFFSETS.latestVersion,
ListOffsetsRequest.DEBUGGING_REPLICA_ID)
+ .forReplica(ApiKeys.LIST_OFFSETS.latestVersion(),
ListOffsetsRequest.DEBUGGING_REPLICA_ID)
Review Comment:
Corrected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1623011371
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Thanks, @CalvinConfluent .
@clolov : In that case, we will need to (1) mark IBP_3_8_IV0 as unused, (2)
create a new MV IBP_4_0_IV0, (3) replace existing references of IBP_3_8_IV0
related to Elr like the following to IBP_4_0_IV0.
```
public boolean isElrSupported() {
return this.isAtLeast(IBP_3_8_IV0);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
CalvinConfluent commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1622979418
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Not yet. We are still finalizing the last PR.
https://github.com/apache/kafka/pull/15702
@clolov I don't want to block your feature, we should let KIP-966 use
another metadata version and mark the IBP_3_8_IV0 as reserved.
@junrao What is your suggestion here? Using a future metadata version like
IBP_4_0_IV0 for KIP-966 at the moment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
CalvinConfluent commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1622977589
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Not yet. We are still finalizing the last PR.
https://github.com/apache/kafka/pull/15702
@clolov I don't want to block your feature, we should mark KIP-966 enabled
for another metadata version.
@junrao What is your suggestion here? Using a future metadata version like
IBP_4_0_IV0 for KIP-966? Also, should I make a change separately or we can do
it in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
CalvinConfluent commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1622977589
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
Not yet. We are still finalizing the last PR.
https://github.com/apache/kafka/pull/15702
@clolov I don't want to block your feature, we should mark KIP-966 enabled
for another metadata version.
@junrao What is your suggestion here? Using a future metadata version like
IBP_4_0_IV0 for KIP-966? Also, should I make a change separately or we can do
it in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1622968644
##
core/src/test/scala/unit/kafka/server/ListOffsetsRequestTest.scala:
##
@@ -55,7 +55,7 @@ class ListOffsetsRequestTest extends BaseRequestTest {
.build()
val debugReplicaRequest = ListOffsetsRequest.Builder
- .forReplica(ApiKeys.LIST_OFFSETS.latestVersion,
ListOffsetsRequest.DEBUGGING_REPLICA_ID)
+ .forReplica(ApiKeys.LIST_OFFSETS.latestVersion(),
ListOffsetsRequest.DEBUGGING_REPLICA_ID)
Review Comment:
We do not, I will raise another version tomorrow morning which address this
+ fix the merge conflicts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1622789214
##
core/src/test/scala/unit/kafka/server/ListOffsetsRequestTest.scala:
##
@@ -55,7 +55,7 @@ class ListOffsetsRequestTest extends BaseRequestTest {
.build()
val debugReplicaRequest = ListOffsetsRequest.Builder
- .forReplica(ApiKeys.LIST_OFFSETS.latestVersion,
ListOffsetsRequest.DEBUGGING_REPLICA_ID)
+ .forReplica(ApiKeys.LIST_OFFSETS.latestVersion(),
ListOffsetsRequest.DEBUGGING_REPLICA_ID)
Review Comment:
Why do we need to add the brackets?
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -228,7 +231,7 @@ public enum MetadataVersion {
* Think carefully before you update this value. ONCE A METADATA
VERSION IS PRODUCTION,
* IT CANNOT BE CHANGED.
*/
-public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4;
+public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV1;
Review Comment:
@CalvinConfluent and @cmccabe : Is IBP_3_8_IV0 for KIP-966 production ready?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2142529825 I have scrapped the unstable flag. I will raise the final changes needed for the CLI command over the weekend - happy for those to be applied on top of the newly cut 3.8 branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2139051996 Heya @junrao, the delay is entirely my bad, I will aim to give an update either today or early tomorrow morning -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2138319133 @clolov Sorry to ping you again. The 3.8 branch is going to be cut by Friday. Will you still be able to complete this PR for 3.8.0? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2123590782 @clolov : 3.8.0 feature freeze is about 1 week away. It would be useful to get this PR in before the 3.8 branch is cut. Any updates on this PR? What's an example of the failed test? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2104991266 @clolov @nikramakrishnan : If an RPC is only used by the client, we don't need to bump up the IBP. However, ListOffsetRequest is used by both the client and the broker. If we don't bump the IBP, we can't test the logic for the new ListOffsetRequest on the broker, right? > Otherwise what happens is that the broker treats the version as non-existent while clients don't respect the configuration and still send the new version. Hmm, normally, a client first sends an ApiVersionRequest to the broker to get exposed API versions. The broker decides whether to expose the latest version based on `unstable.api.versions.enable`. If the broker doesn't expose the latest version, the client shouldn't use it. Also, if somehow the client ignores this and indeed sends the new version, it seems that the broker will still take it. Could you explain the problem a bit more? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
nikramakrishnan commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2104320732 Given that we are marking the latest version as unstable in ListOffsetsResponse, it would make sense to not bump the IBP now as @clolov suggests, and only bump it when ListOffsetsResponse v9 is ready to be marked as stable. Doing this allows us to test v9 with `unstable.api.versions.enable`, and brokers and clients on IV0 will continue to work with v8. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2104306940 Heya @junrao after inspecting some of the tests I think I have misunderstood how marking an API version as unstable works. From a discussion with Nikhil I think we can only do one of two things - we can either keep the IBP and mark the new version as unstable or we can bump the IBP. Otherwise what happens is that the broker treats the version as non-existent while clients don't respect the configuration and still send the new version. Is this your understanding as well? If it is, then I believe I will just mark the version as unstable while I make the client changes and then I will mark it as stable and bump the IBP all in one commit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2100946258 I will look at `kafka.server.ApiVersionsRequestTest` failures tomorrow -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1593670905
##
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##
@@ -717,7 +717,7 @@ public void testUnregisterBroker() throws Throwable {
setBrokerId(0).
setClusterId(active.clusterId()).
setIncarnationId(Uuid.fromString("kxAT73dKQsitIedpiPtwBA")).
-setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV0)).
+setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV1)).
Review Comment:
Hopefully this ought to be addressed in the next commit as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673: URL: https://github.com/apache/kafka/pull/15673#discussion_r1593668135 ## metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java: ## @@ -283,7 +283,7 @@ private static Stream metadataVersionsForTestPartitionRegistration() return Stream.of( MetadataVersion.IBP_3_7_IV1, MetadataVersion.IBP_3_7_IV2, -MetadataVersion.IBP_3_8_IV0 +MetadataVersion.IBP_3_8_IV1 Review Comment: My miss, updated in the next commit -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1592890872
##
metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java:
##
@@ -283,7 +283,7 @@ private static Stream
metadataVersionsForTestPartitionRegistration()
return Stream.of(
MetadataVersion.IBP_3_7_IV1,
MetadataVersion.IBP_3_7_IV2,
-MetadataVersion.IBP_3_8_IV0
+MetadataVersion.IBP_3_8_IV1
Review Comment:
Should we make the same change in PartitionChangeBuilderTest?
##
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##
@@ -717,7 +717,7 @@ public void testUnregisterBroker() throws Throwable {
setBrokerId(0).
setClusterId(active.clusterId()).
setIncarnationId(Uuid.fromString("kxAT73dKQsitIedpiPtwBA")).
-setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV0)).
+setFeatures(brokerFeatures(MetadataVersion.IBP_3_0_IV1,
MetadataVersion.IBP_3_8_IV1)).
Review Comment:
Should we change IBP_3_8_IV0 to IBP_3_8_IV1 in
`testUncleanShutdownBroker()`? It is testing ELR, but we want to make sure ELR
works under the latest version too, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2097975706 Heya @junrao! Let me know your thoughts on my responses. I have futher updated the below + rebased. I am waiting on the tests to finish running and if they uncover something I will aim to remedy it today. MetadataVersionTest - I added 3.8-IV1 where I believe it is needed QuorumControllerTest - I did not add 3.8-IV1 because I believe the references to 3.8-IV0 are to test the ELR feature PartitionRegistrationTest - Updated BrokerMetadataPublisherTest - Updated ZkMigrationIntegrationTest - Updated -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673: URL: https://github.com/apache/kafka/pull/15673#discussion_r1592116050 ## clients/src/main/java/org/apache/kafka/common/requests/ListOffsetsRequest.java: ## @@ -70,7 +70,7 @@ else if (isolationLevel == IsolationLevel.READ_COMMITTED) minVersion = 2; else if (requireTimestamp) minVersion = 1; -return new Builder(minVersion, ApiKeys.LIST_OFFSETS.latestVersion(), CONSUMER_REPLICA_ID, isolationLevel); +return new Builder(minVersion, ApiKeys.LIST_OFFSETS.latestVersion(false), CONSUMER_REPLICA_ID, isolationLevel); Review Comment: We cannot test the latest unstable version in the client, correct. This should be only temporary until I release the subsequent pull request where we introduce a correct OffsetSpec to allow clients to correctly call this behaviour and introduce tests for it -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1592114443
##
core/src/test/scala/unit/kafka/server/ListOffsetsRequestTest.scala:
##
@@ -219,7 +219,7 @@ class ListOffsetsRequestTest extends BaseRequestTest {
TestUtils.generateAndProduceMessages(servers, topic, 9)
TestUtils.produceMessage(servers, topic, "test-10",
System.currentTimeMillis() + 10L)
-for (version <- ApiKeys.LIST_OFFSETS.oldestVersion to
ApiKeys.LIST_OFFSETS.latestVersion) {
+for (version <- ApiKeys.LIST_OFFSETS.oldestVersion to
ApiKeys.LIST_OFFSETS.latestVersion(false)) {
Review Comment:
I want to hide the latest unstable version of the ListOffsetRequest from
everywhere. A follow-up pull request will allow it to be called and introduce
the changes needed for the client to call with this a newer OffsetSpec and a
test confirming the behaviour. Does this make sense or am I misunderstanding
the question?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2090629047 Heya, apologies, I will get to addressing these this weekend! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2089169862 @clolov: Are you able to address the remaining comments? 3.8.0 code freeze is getting close. Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1569221453
##
clients/src/main/java/org/apache/kafka/common/requests/ListOffsetsRequest.java:
##
@@ -70,7 +70,7 @@ else if (isolationLevel == IsolationLevel.READ_COMMITTED)
minVersion = 2;
else if (requireTimestamp)
minVersion = 1;
-return new Builder(minVersion,
ApiKeys.LIST_OFFSETS.latestVersion(), CONSUMER_REPLICA_ID, isolationLevel);
+return new Builder(minVersion,
ApiKeys.LIST_OFFSETS.latestVersion(false), CONSUMER_REPLICA_ID, isolationLevel);
Review Comment:
Hmm, does that mean we can't test the latest unstable version in the client?
##
core/src/test/scala/unit/kafka/server/ListOffsetsRequestTest.scala:
##
@@ -219,7 +219,7 @@ class ListOffsetsRequestTest extends BaseRequestTest {
TestUtils.generateAndProduceMessages(servers, topic, 9)
TestUtils.produceMessage(servers, topic, "test-10",
System.currentTimeMillis() + 10L)
-for (version <- ApiKeys.LIST_OFFSETS.oldestVersion to
ApiKeys.LIST_OFFSETS.latestVersion) {
+for (version <- ApiKeys.LIST_OFFSETS.oldestVersion to
ApiKeys.LIST_OFFSETS.latestVersion(false)) {
Review Comment:
Do we have tests that enable the latest unstable version?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2045111918 Hello @junrao! Yes, I had missed updating some of the tests - I believe the majority of them should now be fixed, but I will circle back once the latest build passes to check again -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
junrao commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2043718730 @clolov : Thanks for the PR. When we last bumped up metadata.version (https://github.com/apache/kafka/pull/14984), we changed a bunch of tests such as MetadataVersionTest, ZkMigrationIntegrationTest, etc to reference 3.8-IV0. Have we added 3.8-IV1 to all needed tests? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov commented on PR #15673: URL: https://github.com/apache/kafka/pull/15673#issuecomment-2042214344 Yup, I need to change the kafka-get-offsets tool to easily access said functionality, but I am in the process of raising that PR -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable [kafka]
clolov opened a new pull request, #15673: URL: https://github.com/apache/kafka/pull/15673 ### Summary This is a follow-up of https://github.com/apache/kafka/pull/15213 where I missed updating the API version and the IBP version. I chose to mark the latest version of the ListOffsets API as unstable until the whole [KIP-1005](https://cwiki.apache.org/confluence/display/KAFKA/KIP-1005%3A+Expose+EarliestLocalOffset+and+TieredOffset) is implemented -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
