KAFKA-16968: Make 3.8-IV0 a stable MetadataVersion and create 3.9-IV0 by cmccabe · Pull Request #16347 · apache/kafka

cmccabe · 2024-06-15T01:11:57Z

Make 3.8-IV0 a stable MetadataVersion
Move KIP-966 out of 3.8, reflecting the fact that it's not ready yet.
Create IBP_3_9_IV0.
Remove IBP_4_0_IV0 since we won't need it until 3.9 is stabilized.

- Make 3.8-IV0 a stable MetadataVersion - Move KIP-966 out of 3.8, reflecting the fact that it's not ready yet. - Remove IBP_4_0_IV0 since we won't need it until 3.9 is stabilized.

dajac

@cmccabe Thanks for the patch. I left a few suggestions.

dajac · 2024-06-17T08:50:28Z

cc @jlprat We need this on in 3.8.

jlprat · 2024-06-17T08:53:10Z

Thanks @dajac! Let's get this to 3.8 once merged

CalvinLiu7947 · 2024-06-17T16:41:02Z

@cmccabe @AndrewJSchofield @dajac , there is another PR adding a new metadata version of 3.8.IV1(it will be the next production ready MV), it also tries to move KIP-966 from 3.8IV0 to 4.0IV0. https://github.com/apache/kafka/pull/15673/files
The PR has been modified for quite some time, If we want to make KIP-966 to 3.9IV0, I can do it in a separate PR.

cmccabe · 2024-06-17T21:15:51Z

@cmccabe @AndrewJSchofield @dajac , there is another PR adding a new metadata version of 3.8.IV1(it will be the next production ready MV), it also tries to move KIP-966 from 3.8IV0 to 4.0IV0. https://github.com/apache/kafka/pull/15673/files
The PR has been modified for quite some time, If we want to make KIP-966 to 3.9IV0, I can do it in a separate PR.

Thanks for pointing out this existing PR. As discussed on the mailnig list, 3.8 is done at this point. No new features are going into it. The changes from #15673 can be done in 3.9-IV0 once this PR creates it.

junrao · 2024-06-17T23:55:32Z

@cmccabe : #15673 is fixing a mistake that shouldn't be in 3.8.0. We should have bumped up the API version for ListOffset, but we didn't. To me, that seems a blocker for 3.8.0, right?

jlprat · 2024-06-18T06:39:20Z

hi @junrao I marked https://issues.apache.org/jira/projects/KAFKA/issues/KAFKA-16480 as a blocker for 3.8

dajac

Thanks for the update. I left a few small comments. Many tests failed too. It looks like that you have to bring back MV 4.0 to a few more places.

cmccabe · 2024-06-18T12:58:33Z

@junrao wrote:

@cmccabe : #15673 is fixing a mistake that shouldn't be in 3.8.0. We should have bumped up the API version for ListOffset, but we didn't. To me, that seems a blocker for 3.8.0, right?

The discussion thread here seems to suggest that #15673 is adding a new feature.:

@junrao : 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?
@clolov: 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

Additionally, the PR title is "KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable #15673". But when I look at clients/src/main/resources/common/message/ListOffsetsRequest.json in the PR, I do not see "latestVersionUnstable": true.

Maybe someone can explain more about what #15673 does exactly.

dajac · 2024-06-18T15:14:04Z

@cmccabe I don't see the last changes. Did you push?

cmccabe · 2024-06-18T15:18:54Z

@dajac Just pushed. I was running a few tests locally as a check.

junrao · 2024-06-18T16:17:23Z

Maybe someone can explain more about what #15673 does exactly.

@cmccabe : The main issue that #15673 fixes
is described in https://issues.apache.org/jira/browse/KAFKA-16480.

https://issues.apache.org/jira/browse/KAFKA-16154 introduced the changes to the ListOffsets API to accept latest-tiered-timestamp and return the corresponding offset.

Those changes should have a) increased the version of the ListOffsets API b) increased the inter-broker protocol version c) hidden the latest version of the ListOffsets behind the latestVersionUnstable flag

So, let's hold off on merging this PR until we understand how to consolidate it with #15673.

cmccabe · 2024-06-18T17:13:22Z

@junrao @clolov @dajac @jlprat : It seems like the client side of the KIP-1005 changes is not implemented. So I would propose we revert #15213 from 3.8 since the KIP is only half-complete, and feature freeze has come and gone. This will unblock the 3.8.0 release.

We can then retarget KIP-1005 at 3.9.0 as usual. As part of that, we can bump the ListOffsets version and mark the latest version of ListOffsets as unstable. This will all be in 3.9-IV0.

I'll update this PR in a moment with those changes

junrao

@cmccabe : Thanks for the PR. Added a few comments.

junrao

@cmccabe : Thanks for the updated PR. A few more comments.

jlprat · 2024-06-20T07:13:47Z

Hi @cmccabe

It seems like the client side of the KIP-1005 changes is not implemented. So I would propose we revert #15213 from 3.8 since the KIP is only half-complete, and feature freeze has come and gone. This will unblock the 3.8.0 release.

Is this the accepted way forward to unlock the 3.8 release? If so, I'll prepare a revert PR.

junrao · 2024-06-20T15:07:09Z

@jlprat : In addition to reverting #15213, it seems that we also need to change LATEST_PRODUCTION to 3.8 in 3.8.0 release.

jlprat · 2024-06-20T17:15:17Z

@junrao I did both in this PR: #16400
(reverting #15213 and changing LATEST_PRODUCTION to IBP_3_8_IV0)

cmccabe · 2024-06-20T17:16:21Z

@jlprat : Is this the accepted way forward to unlock the 3.8 release? If so, I'll prepare a revert PR.

+1

@junrao : In addition to reverting #15213, it seems that we also need to change LATEST_PRODUCTION to 3.8 in 3.8.0 release

Yes. We need to create IBP_3_8_IV0 and mark it as production in 3.8.

junrao

@cmccabe : Thanks for the updated PR. Left a few more comments.

junrao · 2024-06-24T18:19:54Z


  @ClusterTemplate("testApiVersionsRequestValidationV0Template")
-  @ClusterTest(types = Array(Type.KRAFT, Type.CO_KRAFT), metadataVersion = MetadataVersion.IBP_3_7_IV4, serverProperties = Array(
+  @ClusterTest(types = Array(Type.KRAFT, Type.CO_KRAFT), metadataVersion = MetadataVersion.IBP_3_8_IV0, serverProperties = Array(


Could we add a comment that we should use latest production MV for this test?

junrao · 2024-06-24T18:38:45Z

    }

-    @ClusterTest(types = {Type.KRAFT}, metadataVersion = MetadataVersion.IBP_3_7_IV4)
+    @ClusterTest(types = {Type.KRAFT}, metadataVersion = MetadataVersion.IBP_3_8_IV0)


Could we add a comment that we should use latest production MV for this test?

junrao

@cmccabe : Thanks for the updated PR. Just a minor comment.

junrao · 2024-06-24T21:51:28Z

    }

-    @ClusterTest(types = {Type.KRAFT}, metadataVersion = MetadataVersion.IBP_3_7_IV4)
+    @ClusterTest(types = {Type.KRAFT}, metadataVersion = MetadataVersion.IBP_3_8_IV0)


Ok. Could we add a comment on what MV should be used for this test so that it's clear for future developers?

junrao · 2024-06-25T16:18:12Z


    public short listOffsetRequestVersion() {
-        if (this.isAtLeast(IBP_3_5_IV0)) {
+        if (this.isAtLeast(IBP_3_9_IV0)) {


The following test failed because of this. I guess we need to use IBP_3_9_IV0 in the test.

kafka.server.ReplicaFetcherThreadTest.shouldSendLatestRequestVersionsByDefault() org.opentest4j.AssertionFailedError: expected: <9> but was: <8> at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) at app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:134) at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:129) at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:329) at app//kafka.server.ReplicaFetcherThreadTest.shouldSendLatestRequestVersionsByDefault(ReplicaFetcherThreadTest.scala:132)

The following test failed because of this. I guess we need to enable unstable.feature.versions.enable.

org.opentest4j.AssertionFailedError: Expected follower to discover new log start offset 3 at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:38) at app//org.junit.jupiter.api.Assertions.fail(Assertions.java:138) at app//kafka.api.PlaintextAdminIntegrationTest.waitForFollowerLog$1(PlaintextAdminIntegrationTest.scala:846) at app//kafka.api.PlaintextAdminIntegrationTest.testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords(PlaintextAdminIntegrationTest.scala:868) org.apache.kafka.common.errors.InvalidRequestException: Received request api key LIST_OFFSETS with version 9 which is not enabled

org.apache.kafka.tiered.storage.integration.ReassignReplicaExpandTest.executeTieredStorageTest seems to fail consistently now for similar reasons.

org.apache.kafka.common.errors.InvalidRequestException: Received request api key LIST_OFFSETS with version 9 which is not enabled

It seems that you reverted the fix for ReplicaFetcherThreadTest during merging. Also, PlaintextAdminIntegrationTest.testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords still fails.

Both of these tests should pass now.

junrao

@cmccabe : Thanks for the updated PR. A couple of more comments.

junrao · 2024-06-25T22:43:41Z


    public short listOffsetRequestVersion() {
-        if (this.isAtLeast(IBP_3_5_IV0)) {
+        if (this.isAtLeast(IBP_3_9_IV0)) {


It seems that you reverted the fix for ReplicaFetcherThreadTest during merging. Also, PlaintextAdminIntegrationTest.testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords still fails.

junrao · 2024-06-25T22:44:08Z

    }

-    @ClusterTest(types = {Type.KRAFT}, metadataVersion = MetadataVersion.IBP_3_7_IV4)
+    @ClusterTest(types = {Type.KRAFT}, metadataVersion = MetadataVersion.IBP_3_8_IV0)


Was this comment addressed?

jolshan

lgtm as long as the tests look good.

junrao

@cmccabe : Thanks for the updated PR. There are quite a few ApiVersionRequest related test failures.

dajac · 2024-06-28T08:22:33Z

@cmccabe Do we need to cherry-pick it to 3.8 branch too?

jlprat · 2024-06-28T08:23:43Z

@dajac my understanding is that we wouldn't need to cherry pick this to 3.8 but rather have #16400 instead.

dajac · 2024-06-28T08:47:16Z

My understanding is that we still need to create IBP_3_8_IV0 and to update LATEST_PRODUCTION in 3.8 branch. This is orthogonal to #16400.

jlprat · 2024-06-28T08:48:11Z

I added those commits to #16400 as well.
You can use this view for these changes: https://github.com/apache/kafka/pull/16400/files/6acc59d8ff410971530adf2235f0a6943c84c2f0..0058068377996048236f3f494136f315d0960e05

dajac · 2024-06-28T08:54:58Z

Ah, missed them. Then, we're all good. Thanks!

jlprat · 2024-06-28T08:56:04Z

I decided to add both things in the same PR in an attempt to simplify blockers

junrao · 2024-07-01T21:27:32Z

     */
    @ParameterizedTest
-    @ValueSource(strings = {"3.4-IV0", "3.5-IV2", "3.6-IV0", "3.7-IV4"})
+    @ValueSource(strings = {"3.4-IV0", "3.5-IV2", "3.6-IV0", "3.7-IV2"})


@cmccabe : Is it intentional that we leave out "3.9-IV1" in this test?

LATEST_PRODUCTION version in MetadataVersion.java was updated in both apache#16347 and apache#16400, but it was left unchanged in the system tests.

…16533) LATEST_PRODUCTION version in MetadataVersion.java was updated in both #16347 and #16400, but it was left unchanged in the system tests. Reviewers: Josep Prat <josep.prat@aiven.io>

…pache#16533) LATEST_PRODUCTION version in MetadataVersion.java was updated in both apache#16347 and apache#16400, but it was left unchanged in the system tests. Reviewers: Josep Prat <josep.prat@aiven.io>

KAFKA-16968: Make 3.8-IV0 a stable MetadataVersion and create 3.9-IV0

fbb566f

- Make 3.8-IV0 a stable MetadataVersion - Move KIP-966 out of 3.8, reflecting the fact that it's not ready yet. - Remove IBP_4_0_IV0 since we won't need it until 3.9 is stabilized.

dajac reviewed Jun 17, 2024

View reviewed changes

Comment thread server-common/src/main/java/org/apache/kafka/server/common/GroupVersion.java Outdated

Comment thread server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java

AndrewJSchofield reviewed Jun 17, 2024

View reviewed changes

Comment thread server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java

cmccabe mentioned this pull request Jun 17, 2024

KAFKA-16480: Bump ListOffsets version, IBP version and mark last version of ListOffsets as unstable #15673

Closed

re-add 4.0 and add a comment about unstable MVs

ea90660

dajac reviewed Jun 18, 2024

View reviewed changes

Comment thread server-common/src/main/java/org/apache/kafka/server/common/GroupVersion.java Outdated

Comment thread server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java Outdated

fix test

43daca5

Bump ListOffsetsRequest to v9, mark latest version unstable

12370b4

junrao reviewed Jun 18, 2024

View reviewed changes

Comment thread server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java Outdated

cmccabe added 2 commits June 18, 2024 13:23

address review comments

f96af84

fixes

4e9c025

junrao reviewed Jun 18, 2024

View reviewed changes

dajac mentioned this pull request Jun 19, 2024

KAFKA-16192: Introduce transaction.version and usage of flexible records to coordinators #16183

Merged

3 tasks

jlprat mentioned this pull request Jun 20, 2024

Revert "KAFKA-16154" and mark MV 3.8-IV0 as the latest production #16400

Merged

3 tasks

Merge branch 'trunk' into KAFKA-16968

77e8b86

junrao reviewed Jun 24, 2024

View reviewed changes

Add comment

95e1a49

junrao reviewed Jun 24, 2024

View reviewed changes

junrao reviewed Jun 25, 2024

View reviewed changes

cmccabe added 3 commits June 25, 2024 14:32

fix tests

933aed5

Merge branch 'trunk' into KAFKA-16968

a2c7f0a

fix checkstyle

c06985d

junrao reviewed Jun 25, 2024

View reviewed changes

test fixes

c183b4d

jolshan approved these changes Jun 26, 2024

View reviewed changes

junrao reviewed Jun 27, 2024

View reviewed changes

cmccabe added 2 commits June 27, 2024 13:38

Fix ApiVersionsRequestTest

1545da7

Fix SaslApiVersionsRequestTest

fccf7cb

cmccabe force-pushed the KAFKA-16968 branch from 14e2b7e to fccf7cb Compare June 27, 2024 20:56

cmccabe merged commit ebaa108 into apache:trunk Jun 27, 2024

cmccabe deleted the KAFKA-16968 branch June 27, 2024 21:03

junrao reviewed Jul 1, 2024

View reviewed changes

soarez mentioned this pull request Jul 5, 2024

KAFKA-17083: Update LATEST_STABLE_METADATA_VERSION in system tests #16533

Merged

junrao mentioned this pull request Aug 9, 2024

KAFKA-17018: update MetadataVersion for the Kafka release 3.9 #16841

Merged

Conversation

cmccabe commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dajac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dajac commented Jun 17, 2024

Uh oh!

jlprat commented Jun 17, 2024

Uh oh!

Uh oh!

CalvinLiu7947 commented Jun 17, 2024

Uh oh!

cmccabe commented Jun 17, 2024

Uh oh!

junrao commented Jun 17, 2024

Uh oh!

jlprat commented Jun 18, 2024

Uh oh!

dajac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cmccabe commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dajac commented Jun 18, 2024

Uh oh!

cmccabe commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junrao commented Jun 18, 2024

Uh oh!

cmccabe commented Jun 18, 2024

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jlprat commented Jun 20, 2024

Uh oh!

junrao commented Jun 20, 2024

Uh oh!

jlprat commented Jun 20, 2024

Uh oh!

cmccabe commented Jun 20, 2024

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

cmccabe commented Jun 15, 2024 •

edited

Loading

cmccabe commented Jun 18, 2024 •

edited

Loading

cmccabe commented Jun 18, 2024 •

edited

Loading

junrao Jun 25, 2024 •

edited

Loading

jlprat commented Jun 28, 2024 •

edited

Loading