You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "TaiJuWu (via GitHub)" <gi...@apache.org> on 2025/12/11 08:22:02 UTC
[PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
TaiJuWu opened a new pull request, #21127:
URL: https://github.com/apache/kafka/pull/21127
Delete this text and replace it with a detailed description of your change. The
PR title and body will become the squashed commit message.
If you would like to tag individuals, add some commentary, upload images, or
include other supplemental information that should not be part of the eventual
commit message, please use a separate comment.
If applicable, please include a summary of the testing strategy (including
rationale) for the proposed change. Unit and/or integration tests are expected
for any behavior change and system tests should be considered for larger
changes.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "chickenchickenlove (via GitHub)" <gi...@apache.org>.
chickenchickenlove commented on code in PR #21127:
URL: https://github.com/apache/kafka/pull/21127#discussion_r2751251505
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcher.java:
##########
@@ -271,8 +273,23 @@ private void validatePositionsAsync(Map<TopicPartition, FetchPosition> partition
subscriptions.setNextAllowedRetry(fetchPositions.keySet(), nextResetTimeMs);
+ // Convert TopicPartition to TopicIdPartition
+ Map<TopicIdPartition, FetchPosition> fetchPositionsWithId = new HashMap<>();
+ Map<String, Uuid> topicIds = metadata.topicIds();
+ fetchPositions.forEach((tp, position) -> {
+ Uuid topicId = topicIds.get(tp.topic());
+ if (topicId != null) {
+ TopicIdPartition tip = new TopicIdPartition(topicId, tp);
+ fetchPositionsWithId.put(tip, position);
+ } else {
+ // Topic ID not available yet, skip this partition for now
+ // The metadata will be refreshed and we'll retry
+ log.debug("Skipping offset validation for partition {} because topic ID is not available in metadata", tp);
+ }
+ });
Review Comment:
I may be missing some context, but it looks like this path assumes that if a topic doesn’t have a `TopicId` in `metadata.topicIds()`, it will eventually get one and validation will proceed later. In migration / mixed-version environments, I’m wondering if it’s plausible that many topics may not have `TopicIds` for some time (or possibly indefinitely, depending on broker support / metadata propagation).
If that’s the case, should we handle the “missing TopicId” case explicitly here — e.g., fall back to a name-based request (older protocol version such as `v4`), or consider sending `Uuid.ZERO_UUID` for those partitions if that’s the intended compatibility path? Otherwise, my concern is that some `TopicPartitions` could be silently omitted from the `OffsetsForLeaderEpoch` request and remain unvalidated.
Does that concern make sense, or is there an earlier step that guarantees TopicIds will be present by the time we reach this code?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21127:
URL: https://github.com/apache/kafka/pull/21127#issuecomment-4411455337
This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.
<p>
If you are having difficulty finding a reviewer, please reach out on the
[mailing list](https://kafka.apache.org/contact).
<p>
If this PR is no longer valid or desired, please feel free to close it. If no activity
occurs in the next 30 days, it will be automatically closed.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21127:
URL: https://github.com/apache/kafka/pull/21127#issuecomment-3673348111
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "chickenchickenlove (via GitHub)" <gi...@apache.org>.
chickenchickenlove commented on code in PR #21127:
URL: https://github.com/apache/kafka/pull/21127#discussion_r2751252048
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetsRequestManager.java:
##########
@@ -783,8 +785,23 @@ private CompletableFuture<OffsetsForLeaderEpochUtils.OffsetForEpochResult> build
final Node node,
final Map<TopicPartition, SubscriptionState.FetchPosition> fetchPositions,
List<NetworkClientDelegate.UnsentRequest> unsentRequests) {
+ // Convert TopicPartition to TopicIdPartition
+ Map<TopicIdPartition, SubscriptionState.FetchPosition> fetchPositionsWithId = new HashMap<>();
+ Map<String, Uuid> topicIds = metadata.topicIds();
+ fetchPositions.forEach((tp, position) -> {
+ Uuid topicId = topicIds.get(tp.topic());
+ if (topicId != null) {
+ TopicIdPartition tip = new TopicIdPartition(topicId, tp.partition(), tp.topic());
+ fetchPositionsWithId.put(tip, position);
+ } else {
+ // Topic ID not available yet, skip this partition for now
+ // The metadata will be refreshed and we'll retry
+ log.debug("Skipping offset validation for partition {} because topic ID is not available in metadata", tp);
+ }
+ });
Review Comment:
Same as above
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #21127: KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N)
URL: https://github.com/apache/kafka/pull/21127
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu closed pull request #21127: KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N)
URL: https://github.com/apache/kafka/pull/21127
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #21127:
URL: https://github.com/apache/kafka/pull/21127#discussion_r2776716613
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/OffsetFetcher.java:
##########
@@ -271,8 +273,23 @@ private void validatePositionsAsync(Map<TopicPartition, FetchPosition> partition
subscriptions.setNextAllowedRetry(fetchPositions.keySet(), nextResetTimeMs);
+ // Convert TopicPartition to TopicIdPartition
+ Map<TopicIdPartition, FetchPosition> fetchPositionsWithId = new HashMap<>();
+ Map<String, Uuid> topicIds = metadata.topicIds();
+ fetchPositions.forEach((tp, position) -> {
+ Uuid topicId = topicIds.get(tp.topic());
+ if (topicId != null) {
+ TopicIdPartition tip = new TopicIdPartition(topicId, tp);
+ fetchPositionsWithId.put(tip, position);
+ } else {
+ // Topic ID not available yet, skip this partition for now
+ // The metadata will be refreshed and we'll retry
+ log.debug("Skipping offset validation for partition {} because topic ID is not available in metadata", tp);
+ }
+ });
Review Comment:
Nice catch, I always forget we need to support old broker :(
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-10549: Add topic ID support to OffsetForLeader - client (4/N) [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21127:
URL: https://github.com/apache/kafka/pull/21127#issuecomment-4645627373
This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a review.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org