-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-16968: Make 3.8-IV0 a stable MetadataVersion and create 3.9-IV0 #16347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
fbb566f
ea90660
43daca5
12370b4
f96af84
4e9c025
68d1265
84ea68c
77e8b86
95e1a49
933aed5
a2c7f0a
c06985d
c183b4d
1545da7
fccf7cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,7 @@ object ApiVersionsRequestTest { | |
| List(ClusterConfig.defaultBuilder() | ||
| .setTypes(java.util.Collections.singleton(Type.ZK)) | ||
| .setServerProperties(serverProperties) | ||
| .setMetadataVersion(MetadataVersion.IBP_4_0_IV0) | ||
| .setMetadataVersion(MetadataVersion.latestTesting()) | ||
|
cmccabe marked this conversation as resolved.
|
||
| .build()).asJava | ||
| } | ||
|
|
||
|
|
@@ -67,7 +67,7 @@ object ApiVersionsRequestTest { | |
| serverProperties.put("unstable.feature.versions.enable", "false") | ||
| List(ClusterConfig.defaultBuilder() | ||
| .setTypes(java.util.Collections.singleton(Type.ZK)) | ||
| .setMetadataVersion(MetadataVersion.IBP_3_7_IV4) | ||
| .setMetadataVersion(MetadataVersion.latestProduction()) | ||
| .build()).asJava | ||
| } | ||
|
|
||
|
|
@@ -83,7 +83,7 @@ object ApiVersionsRequestTest { | |
| class ApiVersionsRequestTest(cluster: ClusterInstance) extends AbstractApiVersionsRequestTest(cluster) { | ||
|
|
||
| @ClusterTemplate("testApiVersionsRequestTemplate") | ||
| @ClusterTest(types = Array(Type.KRAFT, Type.CO_KRAFT), metadataVersion = MetadataVersion.IBP_4_0_IV0, serverProperties = Array( | ||
| @ClusterTest(types = Array(Type.KRAFT, Type.CO_KRAFT), serverProperties = Array( | ||
| new ClusterConfigProperty(key = "unstable.api.versions.enable", value = "false"), | ||
| new ClusterConfigProperty(key = "unstable.feature.versions.enable", value = "true") | ||
| )) | ||
|
|
@@ -131,8 +131,9 @@ class ApiVersionsRequestTest(cluster: ClusterInstance) extends AbstractApiVersio | |
| assertEquals(ApiKeys.API_VERSIONS.latestVersion(), apiVersion.maxVersion()) | ||
| } | ||
|
|
||
| // Use the latest production MV for this test | ||
| @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( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add a comment that we should use latest production MV for this test?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
| new ClusterConfigProperty(key = "unstable.api.versions.enable", value = "false"), | ||
| new ClusterConfigProperty(key = "unstable.feature.versions.enable", value = "false"), | ||
| )) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,7 +126,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_9_IV1; | ||
|
cmccabe marked this conversation as resolved.
|
||
| default: | ||
| throw new RuntimeException("Unknown PartitionChangeRecord version " + version); | ||
| } | ||
|
|
@@ -313,7 +313,7 @@ public void testNoLeaderEpochBumpIfNothingChanged(short version) { | |
| * Test that shrinking the ISR doesn't increase the leader epoch in later MVs. | ||
| */ | ||
| @ParameterizedTest | ||
| @ValueSource(strings = {"3.6-IV0", "3.7-IV4"}) | ||
| @ValueSource(strings = {"3.6-IV0", "3.7-IV2", "3.9-IV1"}) | ||
| public void testNoLeaderEpochBumpOnIsrShrink(String metadataVersionString) { | ||
| MetadataVersion metadataVersion = MetadataVersion.fromVersionString(metadataVersionString); | ||
| testTriggerLeaderEpochBumpIfNeeded( | ||
|
|
@@ -342,7 +342,7 @@ public void testLeaderEpochBumpOnIsrShrink(String metadataVersionString) { | |
| * Test that shrinking the ISR does increase the leader epoch in later MVs when ZK migration is on. | ||
| */ | ||
| @ParameterizedTest | ||
| @ValueSource(strings = {"3.6-IV0", "3.7-IV4"}) | ||
| @ValueSource(strings = {"3.6-IV0", "3.7-IV2", "3.9-IV1"}) | ||
| public void testLeaderEpochBumpOnIsrShrinkWithZkMigration(String metadataVersionString) { | ||
| MetadataVersion metadataVersion = MetadataVersion.fromVersionString(metadataVersionString); | ||
| testTriggerLeaderEpochBumpIfNeeded( | ||
|
|
@@ -358,7 +358,7 @@ public void testLeaderEpochBumpOnIsrShrinkWithZkMigration(String metadataVersion | |
| * Test that expanding the ISR doesn't increase the leader epoch. | ||
| */ | ||
| @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", "3.9-IV1"}) | ||
| public void testNoLeaderEpochBumpOnIsrExpansion(String metadataVersionString) { | ||
| MetadataVersion metadataVersion = MetadataVersion.fromVersionString(metadataVersionString); | ||
| testTriggerLeaderEpochBumpIfNeeded( | ||
|
|
@@ -372,7 +372,7 @@ public void testNoLeaderEpochBumpOnIsrExpansion(String metadataVersionString) { | |
| * Test that expanding the ISR doesn't increase the leader epoch during ZK migration. | ||
| */ | ||
| @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", "3.9-IV1"}) | ||
| public void testNoLeaderEpochBumpOnIsrExpansionDuringMigration(String metadataVersionString) { | ||
| MetadataVersion metadataVersion = MetadataVersion.fromVersionString(metadataVersionString); | ||
| testTriggerLeaderEpochBumpIfNeeded( | ||
|
|
@@ -389,7 +389,7 @@ public void testNoLeaderEpochBumpOnIsrExpansionDuringMigration(String metadataVe | |
| * always results in a leader epoch increase. | ||
| */ | ||
| @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", "3.9-IV1"}) | ||
| public void testLeaderEpochBumpOnNewReplicaSetDisjoint(String metadataVersionString) { | ||
| MetadataVersion metadataVersion = MetadataVersion.fromVersionString(metadataVersionString); | ||
| testTriggerLeaderEpochBumpIfNeeded( | ||
|
|
@@ -403,7 +403,7 @@ public void testLeaderEpochBumpOnNewReplicaSetDisjoint(String metadataVersionStr | |
| * cannot actually change the ISR, triggerLeaderEpochBumpForIsrShrinkIfNeeded does not engage. | ||
| */ | ||
| @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"}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cmccabe : Is it intentional that we leave out "3.9-IV1" in this test? |
||
| public void testNoLeaderEpochBumpOnEmptyTargetIsr(String metadataVersionString) { | ||
| MetadataVersion metadataVersion = MetadataVersion.fromVersionString(metadataVersionString); | ||
| PartitionRegistration partition = new PartitionRegistration.Builder(). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,11 +202,23 @@ public enum MetadataVersion { | |
| // Add new fetch request version for KIP-951 | ||
| IBP_3_7_IV4(19, "3.7", "IV4", false), | ||
|
|
||
| // New version for the Kafka 3.8.0 release. | ||
| IBP_3_8_IV0(20, "3.8", "IV0", false), | ||
|
|
||
| // | ||
| // NOTE: MetadataVersions after this point are unstable and may be changed. | ||
| // If users attempt to use an unstable MetadataVersion, they will get an error. | ||
| // Please move this comment when updating the LATEST_PRODUCTION constant. | ||
| // | ||
|
|
||
| // Support ListOffsetRequest v9 for KIP-1005. | ||
| IBP_3_9_IV0(21, "3.9", "IV0", false), | ||
|
|
||
| // Add ELR related supports (KIP-966). | ||
|
cmccabe marked this conversation as resolved.
|
||
| IBP_3_8_IV0(20, "3.8", "IV0", true), | ||
| IBP_3_9_IV1(22, "3.9", "IV1", true), | ||
|
|
||
| // Introduce version 1 of the GroupVersion feature (KIP-848). | ||
| IBP_4_0_IV0(21, "4.0", "IV0", false); | ||
| IBP_4_0_IV0(23, "4.0", "IV0", false); | ||
|
|
||
| // NOTES when adding a new version: | ||
| // Update the default version in @ClusterTest annotation to point to the latest version | ||
|
|
@@ -232,7 +244,7 @@ public enum MetadataVersion { | |
| * <strong>Think carefully before you update this value. ONCE A METADATA VERSION IS PRODUCTION, | ||
| * IT CANNOT BE CHANGED.</strong> | ||
| */ | ||
| public static final MetadataVersion LATEST_PRODUCTION = IBP_3_7_IV4; | ||
| public static final MetadataVersion LATEST_PRODUCTION = IBP_3_8_IV0; | ||
|
cmccabe marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * An array containing all of the MetadataVersion entries. | ||
|
|
@@ -331,7 +343,7 @@ public boolean isDirectoryAssignmentSupported() { | |
| } | ||
|
|
||
| public boolean isElrSupported() { | ||
| return this.isAtLeast(IBP_3_8_IV0); | ||
| return this.isAtLeast(IBP_3_9_IV1); | ||
| } | ||
|
|
||
| public boolean isKRaftSupported() { | ||
|
|
@@ -459,7 +471,9 @@ public short offsetForLeaderEpochRequestVersion() { | |
| } | ||
|
|
||
| public short listOffsetRequestVersion() { | ||
| if (this.isAtLeast(IBP_3_5_IV0)) { | ||
| if (this.isAtLeast(IBP_3_9_IV0)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that you reverted the fix for ReplicaFetcherThreadTest during merging. Also, PlaintextAdminIntegrationTest.testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords still fails.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both of these tests should pass now. |
||
| return 9; | ||
|
cmccabe marked this conversation as resolved.
|
||
| } else if (this.isAtLeast(IBP_3_5_IV0)) { | ||
| return 8; | ||
| } else if (this.isAtLeast(IBP_3_0_IV1)) { | ||
| return 7; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.