-
Notifications
You must be signed in to change notification settings - Fork 14.8k
KAFKA-19802: Admin client changes for KIP-1226 #20771
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
Conversation
lianetm
left a comment
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! Just nits on the comments, LGTM.
| private final Optional<Long> lag; | ||
|
|
||
| /** | ||
| * Construct a new StartPartitionOffsetInfo. |
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.
| * Construct a new StartPartitionOffsetInfo. | |
| * Construct a new SharePartitionOffsetInfo. |
| * Construct a new StartPartitionOffsetInfo. | ||
| * | ||
| * @param startOffset The share-partition start offset | ||
| * @param leaderEpoch The optional leader epoch of the start offset |
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.
I expect this is the leader epoch of the share partition, correct?
chia7712
left a comment
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.
@AndrewJSchofield thanks for this patch. I have two small questions. Please take a look.
| groupOffsetsListing.put(tp, null); | ||
| } else { | ||
| groupOffsetsListing.put(tp, new OffsetAndMetadata(startOffset, leaderEpoch, "")); | ||
| groupOffsetsListing.put(tp, new SharePartitionOffsetInfo(startOffset, leaderEpoch, Optional.empty())); |
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.
Pardon me, will the lag be implemented later?
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.
Yes. Very soon. https://issues.apache.org/jira/browse/KAFKA-19778 for the tasks we are implementing in parallel.
| /** | ||
| * Construct a new SharePartitionOffsetInfo. | ||
| * | ||
| * @param startOffset The share-partition start offset |
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.
Sorry, I'm a bit confused by this "start offset". For example, If a partition has a latest offset of 23547540 and a share consumer has a "start offset" of 23541472, how do we explain the difference between those numbers?
chia7712@fedora:~/project/kafka$ ./bin/kafka-get-offsets.sh --bootstrap-server localhost:20000 --topic chia
chia:0:23547540
chia7712@fedora:~/project/kafka$ ./bin/kafka-share-groups.sh --bootstrap-server localhost:20000 --offsets --describe --all-groups
GROUP TOPIC PARTITION START-OFFSET LAG
perf-share-consumer chia 0 23541472 -
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.
The start offset is a little like the committed offset for a consumer group. The lag is the number of records to be delivered between the start offset and the latest offset.
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.
@AndrewJSchofield thanks for your response! I have a follow-up question.
The start offset is a little like the committed offset for a consumer group.
Will the start offset eventually equal to the latest offset of the partition?
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.
When consumption catches up with production, yes.
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.
The start offset is a little like the committed offset for a consumer group.
The values for the committed offset and start offset seem different. My understanding is that the committed offset is the next offset to be read, whereas the start offset is the last offset that was read.
For example, if we:
- start a consumer and a share consumer
- put one record (offset = 0) to the topic
- the committed offset is 1
- the start offset is 0
Please correct me if I misunderstand anything 😃
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.
You are correct. I did say "a little like". The start offset is basically the first offset which the share partition is considering for delivery. With start offset and lag, we are expressing progress delivering records and a measure of the records whose delivery has not yet completed.
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.
we are expressing progress delivering records and a measure of the records whose delivery has not yet completed.
Pardon me, the start offset is 0 even though the offset is consumed and committed by share consumer? Is it correct?
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.
The update of the start offset is lazy for performance reasons. Sometimes, it's very slightly behind where you might expect, but next time we write the start offset to the __share_group_state topic, it moves forwards. As long as the delivery state of the record is accurate, having the SPSO a little behind doesn't matter from a logical standpoint.
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.
@AndrewJSchofield thanks for all your explanation!
Admin client changes for KIP-1226 which adds lag information for share groups. Reviewers: Lianet Magrans <lmagrans@confluent.io>
Admin client changes for KIP-1226 which adds lag information for share
groups.
Reviewers: Lianet Magrans lmagrans@confluent.io