KAFKA-10547; add topicId in MetadataResp by dengziming · Pull Request #9622 · apache/kafka

dengziming · 2020-11-19T11:59:14Z

More detailed description of your change

Bump the version of MetadataReq and MetadataResp, add topicId in MetadataResp
Alter describeTopic in AdminClientTopicService and ZookeeperTopicService
TopicMetadata is cached in MetadataCache, so we need to add topicId to MetadataCache
MetadataCache is updated by UpdateMetadataRequest, bump the version of UpdateMetadataReq and UpdateMetadataResp, add topicId in UpdateMetadataReq.

Summary of testing strategy (including rationale)

Tested locally, here is some result:

New server + new Client :

kafka-topics.sh --describe --zookeeper localhost:2181 --topic old-version-topic
Topic: old-version-topic TopicId: wRPl6VAlQeyE77bDxEESzg PartitionCount: 2 ReplicationFactor: 1 Configs:
Topic: old-version-topic Partition: 0 Leader: 0 Replicas: 0 Isr: 0
Topic: old-version-topic Partition: 1 Leader: 0 Replicas: 0 Isr: 0

kafka-topics.sh --describe --bootstrap-server localhost:9092 --topic old-version-topic
Topic: old-version-topic TopicId: wRPl6VAlQeyE77bDxEESzg PartitionCount: 2 ReplicationFactor: 1 Configs: segment.bytes=1073741824
Topic: old-version-topic Partition: 0 Leader: 0 Replicas: 0 Isr: 0
Topic: old-version-topic Partition: 1 Leader: 0 Replicas: 0 Isr: 0

Old Server + new Client

kafka-topics.sh --describe --bootstrap-server localhost:9092 --topic old-version-topic
Topic: old-version-topic PartitionCount: 2 ReplicationFactor: 1 Configs: segment.bytes=1073741824
Topic: old-version-topic Partition: 0 Leader: 0 Replicas: 0 Isr: 0
Topic: old-version-topic Partition: 1 Leader: 0 Replicas: 0 Isr: 0

New server + old client

kafka-topics.sh --describe --bootstrap-server localhost:9092 --topic old-version-topic
Topic: old-version-topic PartitionCount: 2 ReplicationFactor: 1 Configs: segment.bytes=1073741824
Topic: old-version-topic Partition: 0 Leader: 0 Replicas: 0 Isr: 0
Topic: old-version-topic Partition: 1 Leader: 0 Replicas: 0 Isr: 0

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

dengziming · 2020-11-19T12:12:28Z

@rajinisivaram @jolshan Hi, PTAL.

jolshan · 2020-11-19T23:53:47Z

Hi @dengziming! Thanks for the PR! I was hoping to add LeaderAndIsrRequests before UpdateMetadata/Metadata, following the ordering of the JIRA tickets. There are just a few features for persisting the topic IDs I wanted to include. I'm thinking we could review this PR and my PR: #9626 at the same time and merge mine first and yours immediately after.

clients/src/main/java/org/apache/kafka/common/Cluster.java

core/src/main/scala/kafka/admin/TopicCommand.scala

core/src/main/scala/kafka/api/ApiVersion.scala

jolshan · 2020-11-22T21:11:33Z

@dengziming Looks pretty good so far to me! I think it would be useful to write a few unit/integration tests to ensure the metadata snapshot behavior and describe topics work as expected.

clients/src/main/resources/common/message/MetadataRequest.json

dengziming · 2020-11-25T03:34:38Z

@dengziming Looks pretty good so far to me! I think it would be useful to write a few unit/integration tests to ensure the metadata snapshot behavior and describe topics work as expected.

I added/altered some unit/integration tests in MetadataRequestTest and ControllerChannelManagerTest, take a look.

rajinisivaram

@dengziming Thanks for the PR, looks good. Left some comments.

core/src/main/scala/kafka/log4j.properties

core/src/main/scala/kafka/server/MetadataCache.scala

core/src/test/scala/unit/kafka/controller/ControllerChannelManagerTest.scala

clients/src/main/java/org/apache/kafka/common/Cluster.java

core/src/main/scala/kafka/server/MetadataCache.scala

rajinisivaram

@dengziming Thanks for the updates, looks good, just a couple more comments left.

clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java

core/src/test/scala/unit/kafka/server/MetadataRequestTest.scala

core/src/main/scala/kafka/server/MetadataCache.scala

rajinisivaram

@dengziming Thanks for the update, just one minor comment left, apart from that LGTM.

clients/src/main/java/org/apache/kafka/common/Cluster.java

KAFKA-10547; alter some integration tests KAFKA-10547; optimeze imports resolve comments

rajinisivaram

@dengziming Thanks for the update, LGTM. Will review PR #9626 as well and merge these two together, hopefully later today.

rajinisivaram · 2020-12-18T18:14:07Z

There is a compilation failure in the Java 8 build because the PR uses an API not available in Scala 2.12. Will push a fix.

rajinisivaram · 2020-12-18T21:28:25Z

@dengziming Thanks for the PR, merging to trunk.

jolshan mentioned this pull request Nov 19, 2020

KAFKA-10545: Create topic IDs and propagate to brokers #9626

Merged

3 tasks

dengziming force-pushed the KAFKA-10547 branch from aefa813 to af6e8a1 Compare November 20, 2020 12:15

jolshan reviewed Nov 22, 2020

View reviewed changes

clients/src/main/java/org/apache/kafka/common/Cluster.java Outdated Show resolved Hide resolved

jolshan reviewed Nov 22, 2020

View reviewed changes

core/src/main/scala/kafka/admin/TopicCommand.scala Outdated Show resolved Hide resolved

jolshan reviewed Nov 22, 2020

View reviewed changes

core/src/main/scala/kafka/api/ApiVersion.scala Outdated Show resolved Hide resolved

jolshan reviewed Nov 22, 2020

View reviewed changes

clients/src/main/resources/common/message/MetadataRequest.json Outdated Show resolved Hide resolved

dengziming force-pushed the KAFKA-10547 branch from af6e8a1 to 54a2236 Compare November 23, 2020 13:53

dengziming force-pushed the KAFKA-10547 branch from aa82c71 to 6b3a0e6 Compare December 11, 2020 05:18

rajinisivaram reviewed Dec 15, 2020

View reviewed changes

dengziming requested a review from rajinisivaram December 16, 2020 06:06

rajinisivaram reviewed Dec 16, 2020

View reviewed changes

dengziming force-pushed the KAFKA-10547 branch 2 times, most recently from 5e59937 to 680308e Compare December 17, 2020 03:17

dengziming requested a review from rajinisivaram December 17, 2020 03:32

rajinisivaram reviewed Dec 17, 2020

View reviewed changes

clients/src/main/java/org/apache/kafka/common/Cluster.java Outdated Show resolved Hide resolved

KAFKA-10547; alter some unit tests

587c0e0

KAFKA-10547; alter some integration tests KAFKA-10547; optimeze imports resolve comments

dengziming force-pushed the KAFKA-10547 branch from 680308e to 587c0e0 Compare December 17, 2020 10:41

dengziming requested a review from rajinisivaram December 17, 2020 10:43

rajinisivaram approved these changes Dec 17, 2020

View reviewed changes

Fix Scala 2.12 build

b48aa7f

rajinisivaram merged commit 5c921af into apache:trunk Dec 18, 2020

dengziming mentioned this pull request Apr 21, 2021

KAFKA-12701: Remove topicId from MetadataReq since it was not supported in 2.8.0 #10576

Closed

3 tasks

dengziming deleted the KAFKA-10547 branch October 8, 2022 11:51

Conversation

dengziming commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

dengziming commented Nov 19, 2020

Uh oh!

jolshan commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jolshan commented Nov 22, 2020

Uh oh!

Uh oh!

dengziming commented Nov 25, 2020

Uh oh!

rajinisivaram left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajinisivaram left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajinisivaram left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rajinisivaram left a comment

Choose a reason for hiding this comment

Uh oh!

rajinisivaram commented Dec 18, 2020

Uh oh!

rajinisivaram commented Dec 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

dengziming commented Nov 19, 2020 •

edited

Loading

jolshan commented Nov 19, 2020 •

edited

Loading