-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
KAFKA-15922: Add a MetadataVersion for JBOD #14860
Conversation
Assign MetadataVersion.IBP_3_7_IV2 to JBOD. Move KIP-966 support to MetadataVersion.IBP_3_7_IV3. Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable and unstable metadata versions for the first time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also fix the builder version handling here?
https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/requests/BrokerRegistrationRequest.java#L48
https://github.com/apache/kafka/blob/trunk/clients/src/test/java/org/apache/kafka/common/requests/BrokerRegistrationRequestTest.java#L56
With this fixed, LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments, nothing major directly on the changes.
I do have a high-level thought to put out there. This currently prevents the storage tool from formatting with a non-production MetadataVersion
and prevents the feature tool from updating a running cluster to a non-production MetadataVersion. I wonder if this will break anything, and I wonder if perhaps we should add a break-glass mechanism to circumvent these things in case something does break. While this may not require a KIP per-se, I would not be surprised if changing this causes issues for people somewhere.
metadata/src/main/resources/common/metadata/PartitionRecord.json
Outdated
Show resolved
Hide resolved
metadata/src/main/resources/common/metadata/PartitionRecord.json
Outdated
Show resolved
Hide resolved
server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java
Show resolved
Hide resolved
server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java
Outdated
Show resolved
Hide resolved
metadata/src/main/java/org/apache/kafka/controller/QuorumFeatures.java
Outdated
Show resolved
Hide resolved
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the changes. The internal break-glass config is a good addition. LGTM except for Calvin's points raised at #14860 (review)
Good catch. The best way to fix this would be to add "ignorable" to the JSON definition, so that it will get automatically dropped if the version is too old. Will do. |
Assign MetadataVersion.IBP_3_7_IV2 to JBOD. Move KIP-966 support to MetadataVersion.IBP_3_7_IV3. Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable and unstable metadata versions for the first time. Reviewers: Igor Soarez <[email protected]>, Ron Dagostino <[email protected]>, Calvin Liu <[email protected]>, Proven Provenzano <[email protected]>
if (version < 2) { | ||
// Reset the PreviousBrokerEpoch to default if not supported. | ||
data.setPreviousBrokerEpoch(new BrokerRegistrationRequestData().previousBrokerEpoch()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmccabe I am not an expert with the change in the PR but this change might have broken the tests for BrokerRegistrationRequestTest
, tests fails for version < 2
Also I might be missing something to read the builds for this PR but as far as I can check I don't see any successful build for the PR as well, please let me know if I am reading something wrong.
https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/activity?branch=PR-14860
cc: @junrao @pprovenzano @dajac @AndrewJSchofield (for failing tests in the other PR builds for BrokerRegistrationRequestTest
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this is being tracked in #14887.
Assign MetadataVersion.IBP_3_7_IV2 to JBOD. Move KIP-966 support to MetadataVersion.IBP_3_7_IV3. Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable and unstable metadata versions for the first time. Reviewers: Igor Soarez <[email protected]>, Ron Dagostino <[email protected]>, Calvin Liu <[email protected]>, Proven Provenzano <[email protected]>
Assign MetadataVersion.IBP_3_7_IV2 to JBOD. Move KIP-966 support to MetadataVersion.IBP_3_7_IV3. Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable and unstable metadata versions for the first time. Reviewers: Igor Soarez <[email protected]>, Ron Dagostino <[email protected]>, Calvin Liu <[email protected]>, Proven Provenzano <[email protected]>
Assign MetadataVersion.IBP_3_7_IV2 to JBOD. Move KIP-966 support to MetadataVersion.IBP_3_7_IV3. Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable and unstable metadata versions for the first time. Reviewers: Igor Soarez <[email protected]>, Ron Dagostino <[email protected]>, Calvin Liu <[email protected]>, Proven Provenzano <[email protected]>
Assign MetadataVersion.IBP_3_7_IV2 to JBOD. Move KIP-966 support to MetadataVersion.IBP_3_7_IV3. Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable and unstable metadata versions for the first time. Reviewers: Igor Soarez <[email protected]>, Ron Dagostino <[email protected]>, Calvin Liu <[email protected]>, Proven Provenzano <[email protected]>
Assign MetadataVersion.IBP_3_7_IV2 to JBOD.
Move KIP-966 support to MetadataVersion.IBP_3_7_IV3.
Create MetadataVersion.LATEST_PRODUCTION as the latest metadata version that can be used when formatting a new cluster, or upgrading a cluster using kafka-features.sh. This will allow us to clearly distinguish between stable and unstable metadata versions for the first time.