hi PoAn
thanks for this kip. A couple of questions are listed below.
chia_0:
Should `listConfigs` be renamed to `listConfigResources` to match the returned
type?
chia_1:
v0 returns only the ClientMetricsResource, right? If so, could you please add
docs to the protocol file and KIP
chia_2:
Should we rollback to use v0 when the request fails due to version match? I
point that because users are suggested to use new method instead of
listClientMetricsResources, and hence they may encounter version error if not
all nodes are up-to-date.
chia_3:
`ListClientMetricsResourcesRequest` and `ListClientMetricsResourcesResponse`
are not public APIs, so it seems to me it is unnecessary to "deprecate" them
chia_4:
could you please add "empty means all sources are returned" to
ListConfigResourcesRequest.json?
Best,
Chia-Ping
On 2025/04/02 02:10:31 PoAn Yang wrote:
> Hi all,
>
> If there is no further discussion, I will start vote thread for KIP-1142
> tomorrow.
>
> Thanks,
> PoAn
>
> > On Mar 16, 2025, at 5:23 PM, PoAn Yang <yangp...@gmail.com> wrote:
> >
> > Hi Andrew,
> >
> > Thanks for the review.
> >
> > AS1: The kafka-client-metrics.sh —describe —name for a non-existent
> > client-metric shows default configuration.
> > The behavior is inconsistent with kafka-configs.sh in this KIP. I add this
> > case to proposed changes as well. Thanks.
> >
> > AS2: Thanks for the great suggestion. We have an API DescribeConfig to get
> > all kinds of configuration.
> > It’s better to have a API to list all types of configuration. I change
> > ListClientMetricsResources to ListConfigResources
> > and remove change for ListGroups. For backward compatibility, we also need
> > to make sure v0 ListConfigResources
> > can work as ListClientMetricsResources.
> >
> > AS3: Without change for ListGroupsResponse, we don’t need to handle state
> > and type fields in this KIP.
> >
> > AS4: Remove change for ListGroupsResponse.
> >
> > Thanks,
> > PoAn
> >
> >> On Mar 14, 2025, at 6:37 PM, Andrew Schofield
> >> <andrew_schofield_j...@outlook.com> wrote:
> >>
> >> Hi PoAn,
> >> Thanks for the KIP. It would certainly be good to fix this slightly odd
> >> behaviour in Kafka configs.
> >>
> >> AS1: I'd like to confirm that the KIP also alters the behaviour of
> >> kafka-client-metrics.sh --describe
> >> for the case where the resource does not exist. You do show this in an
> >> example, but I wanted to
> >> confirm that this reflects a behaviour change that this KIP will enact.
> >>
> >> AS2: I wonder whether there's an alternative way to do this in the
> >> protocol. In the way that
> >> you have specified it, if ListGroups specified
> >> IncludeNonExistentGroupWithDynamicConfig,
> >> the returned list of groups comes from two sources - the groups in the
> >> group coordinator
> >> and the group config resources in the metadata.
> >>
> >> I wonder whether actually you could rename the ListClientMetricsResources
> >> RPC to
> >> ListConfigResources, bump it to version 1 with a schema like this:
> >>
> >> {
> >> "apiKey": 74,
> >> "type": "request",
> >> "listeners": ["broker"],
> >> "name": "ListConfigResourcesRequest",
> >> "validVersions": "0-1",
> >> "flexibleVersions": "0+",
> >> "fields": [
> >> { "name": "ResourceType", "type": "int8", "versions": "1+", "mapKey":
> >> true,
> >> "about": "The resource type." }
> >> ]
> >> }
> >>
> >> For v0, the resource type is client metrics.
> >>
> >> Then, the response would be:
> >>
> >> {
> >> "apiKey": 74,
> >> "type": "response",
> >> "name": "ListConfigResourcesResponse",
> >> "validVersions": "0-1",
> >> "flexibleVersions": "0+",
> >> "fields": [
> >> { "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
> >> "about": "The duration in milliseconds for which the request was
> >> throttled due to a quota violation, or zero if the request did not violate
> >> any quota." },
> >> { "name": "ErrorCode", "type": "int16", "versions": "0+",
> >> "about": "The error code, or 0 if there was no error." },
> >> { "name": "ConfigResources", "type": "[]ConfigResource", "versions":
> >> "0+",
> >> "about": "Each resource in the response.", "fields": [
> >> { "name": "Name", "type": "string", "versions": "0+",
> >> "about": "The resource name." }
> >> ]}
> >> ]
> >> }
> >>
> >> Then, this could list the group config resources (which is not the same as
> >> listing the existing groups):
> >>
> >>> bin/kafka-configs.sh --describe --entity-type groups
> >>
> >> And perhaps, this could list the config for the existing groups because it
> >> uses ListGroups to get the list of groups:
> >>
> >>> bin/kafka-configs.sh --describe --entity-type groups --effective
> >>
> >> The advantage to having ListConfigResources is that it would apply to
> >> future resources too without needing
> >> additional RPCs. It also lets you distinguish between what's configuration
> >> and what's live.
> >>
> >> Just a random idea for your consideration.
> >>
> >> AS3: A group which does not exist but is only represented by a
> >> configuration resource has
> >> neither a type nor a state. What values will be used for these fields in
> >> the ListGroupsResponse.
> >>
> >> AS4: The comment in ListGroupsResponse for v6 is incorrect. v6 adds the
> >> NonExistentGroupWithDynamicConfig field.
> >>
> >>
> >> Thanks,
> >> Andrew
> >>
> >> ________________________________________
> >> From: PoAn Yang <yangp...@gmail.com>
> >> Sent: 12 March 2025 16:00
> >> To: dev@kafka.apache.org <dev@kafka.apache.org>
> >> Subject: [DISCUSS] KIP-1142: Allow to list non-existent group which has
> >> dynamic config
> >>
> >> Hi all,
> >>
> >> I would like to start a discussion thread on KIP-1142.
> >>
> >> Currently, kafka-configs.sh requires explicit group names to describe
> >> configurations, which limits flexibility in managing dynamically
> >> configured groups. This KIP proposes extending the ListGroups API to
> >> include groups that do not exist in the coordinator but have dynamic
> >> configurations.
> >>
> >> Please take a look and feel free to share any thoughts.
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1142%3A+Allow+to+list+non-existent+group+which+has+dynamic+config
> >>
> >> Thanks,
> >> PoAn
> >
>
>
