Hi, David,
Thanks for the reply and the updated KIP. A few more comments on the
interfaces and the protocols.
60. On the consumer side, do we need both PartitionAssignor.onAssignment
and ConsumerRebalanceListener.onPartitionsAssigned? My understanding is
that the former was added for cooperative rebalance, which is now handled
by the coordinator. If we do need both, should we make them more consistent
(e.g. topic name vs topic id, list vs set vs collection)?
61. group.local.assignors: Could we make it clear that it's the full class
name that implements PartitionAssignor?
62. MemberAssignment: It currently has the following method.
public Set<TopicPartition> topicPartitions()
We are adding List<TopicIdPartition> ownedPartitions. Should we keep the
naming and the return type consistent?
63. MemberAssignment.error: should that be reason?
64. group.remote.assignor: The client may not know what assignors the
broker supports. Should we default this to what the broker determines (e.g.
first assignor listed in group.consumer.assignors)?
65. After the text "When A heartbeats again and acknowledges the
revocation, the group coordinator transitions him to epoch 2 and releases
foo-2.", we have the following.
B - epoch=2, partitions=[foo-2], pending-partitions=[]
Should foo-2 be in pending-partitions?
66. In the Online Migration example, is the first occurence of "C -
epoch=23, partitions=[foo-2, foo-5, foo-4], pending-partitions=[]" correct?
It seems that should happen after C receives a SyncGroup response? If so,
subsequent examples have the same issue.
67. ConsumerGroupHeartbeatRequest.RebalanceTimeoutMs : Which config
controls this? How is this used by the group coordinator since there is no
sync barrier anymore?
68. ConsumerGroupHeartbeatResponse:
68.1 AssignedTopicPartitions and PendingTopicPartitions are of type
[]TopicPartition. Should they be TopicPartitions?
68.2 Assignment.error. Should we also have an errorMessage field?
69. ConsumerGroupPrepareAssignmentResponse.Members.Assignor: Should it
include the selected assignor name?
70. ConsumerGroupInstallAssignmentRequest.GroupEpoch: Should we let the
client set this? Intuitively, it seems the coordinator should manage the
group epoch.
71. ConsumerGroupDescribeResponse:
71.1 Members.Assignment.Partitions. Should we include the topic name too
since it's convenient for building tools? Ditto for TargetAssignment.
71.2 Members: Should we include SubscribedTopicRegex too?
72. OffsetFetchRequest: Is GenerationIdOrMemberEpoch needed since tools may
also want to issue this request?
Thanks,
Jun
