You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Gantigmaa Selenge <gs...@redhat.com> on 2024/07/25 13:19:28 UTC
[DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Hi everyone,
I would like to start a discussion on KIP-1073 that includes inactive
observer nodes in the response for describeQuorum request.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
The initial discussion on this issue is here, if you would like to see what
was previously discussed:
https://lists.apache.org/list.html?dev@kafka.apache.org
Any feedback and suggestions for the KIP are welcome in this email thread.
Thank you.
Regards,
Gantigmaa Selenge
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Colin McCabe <cm...@apache.org>.
Thanks, Gantigmaa. The new KIP looks better.
The admin client changes don't seem to include a way to find out if a given broker is fenced, by examining the result. Seems like we need that as well?
> Newer Admin clients connecting to older brokers will use the older protocol version and hence will not
> request fenced brokers.
You need to describe what happens when the client requests to see fenced brokers, but the protocol doesn't support that. I would suggest UnsupportedVersionException.
best,
Colin
On Mon, Aug 19, 2024, at 08:29, Gantigmaa Selenge wrote:
> Hi everyone,
>
> I have updated the proposed solution of the KIP. Instead of extending the
> DescribeQuorum API, it now suggests updating the DescribeCluster API to
> optionally return fenced brokers. This should address the comments from
> Colin and Jose.
>
> Please let me know if you have any questions.
>
> Regards,
> Gantigmaa
>
> On Thu, Aug 15, 2024 at 9:00 AM Gantigmaa Selenge <gs...@redhat.com>
> wrote:
>
>> Hi Colin and Jose,
>>
>> Thanks for taking a look at the KIP and providing feedback.
>>
>> I overlooked that the memory state containing observers will be lost when
>> a controller restarts. You also made a good point about other observers
>> that might be connecting to the quorum to read the current metadata. I
>> agree that we should implement a way to list the fenced brokers using the
>> DescribeClusterRequest instead of the DescribeQuorum API.
>>
>> I will update the KIP and get back to you all for further feedback. Thanks.
>>
>> Regards,
>> Gantigmaa
>>
>> On Fri, Aug 9, 2024 at 11:48 PM José Armando García Sancio
>> <js...@confluent.io.invalid> wrote:
>>
>>> Thanks for the KIP Gantigmaa!
>>>
>>> I agree with the motivation but it is not clear to me that this should be
>>> solved in the KRaft layer. The KRaft leader only keeps track, in-memory,
>>> of
>>> observers that have fetched. It is possible, after a kraft leader change,
>>> for this state to get lost. If the shutdown brokers never fetch they will
>>> not show up in the kraft leader's observer list.
>>>
>>> As Colin has pointed out it is better to solve this in the metadata module
>>> and the KRaft/Quorum controller. The QuorumController knows of all of the
>>> registered brokers and their state: fence, unfence and even the heartbeat
>>> state. Take a look at
>>> org.apache.kafka.controller.BrokerHeartbeatManager.BrokerHeartbeatState
>>> for
>>> some of the heartbeat state tracked.
>>>
>>> To expose this information we can extend the kafka-cluster CLI, and also
>>> extend Metadata RPC or introduce a new RPC that returns more broker level
>>> information.
>>>
>>> Thanks,
>>>
>>> On Thu, Jul 25, 2024 at 9:21 AM Gantigmaa Selenge <gs...@redhat.com>
>>> wrote:
>>>
>>> > Hi everyone,
>>> >
>>> > I would like to start a discussion on KIP-1073 that includes inactive
>>> > observer nodes in the response for describeQuorum request.
>>> >
>>> >
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
>>> >
>>> > The initial discussion on this issue is here, if you would like to see
>>> what
>>> > was previously discussed:
>>> > https://lists.apache.org/list.html?dev@kafka.apache.org
>>> >
>>> > Any feedback and suggestions for the KIP are welcome in this email
>>> thread.
>>> >
>>> > Thank you.
>>> > Regards,
>>> > Gantigmaa Selenge
>>> >
>>>
>>>
>>> --
>>> -José
>>>
>>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by José Armando García Sancio <js...@confluent.io.INVALID>.
Thanks for the updates. The latest KIP LGTM.
On Fri, Sep 20, 2024 at 8:38 AM Gantigmaa Selenge <gs...@redhat.com> wrote:
>
> Hi everyone,
>
> Thanks for reviewing the KIP again! I have updated it to address the latest
> comments from Colin and Jose.
>
> Going back to Jose's earlier comment here:
> > Did you consider making "includeFencedBroker" field value implicitly
> based on the DescribeClusterRequest version and the EndpointType? For
> example, if the request version is 2 and the endpoint type is broker,
> the response will include all of the brokers and if they are fenced or
> unfenced. I am suggesting this because a request of endpointType
> controller and includeFencedBroker true doesn't make sense. In
> general, users of the AdminClient cannot easily discover the API
> versions supported by the destination node since the destination node
> may not be known when the admin call is made.
>
> I think admins would request fenced brokers only in certain scenarios, so
> perhaps it makes sense to include them only when requested via the new
> option. If the endpoint type is controller and the option to include fenced
> brokers set to true, the Admin Client can throw an
> "IllegalArgumentException" without sending the request to the controller.
>
> If you are happy with the updates I made, I would like to start the vote on
> this KIP. Thank you.
>
> Regards,
> Gantigmaa
>
>
> On Sat, Sep 14, 2024 at 9:51 PM Colin McCabe <cm...@apache.org> wrote:
>
> > Hi Gantigmaa Selenge,
> >
> > I think we should have a boolean in the request like "includeFenced" which
> > defaults to false. Then if we can't include the fenced brokers, we throw an
> > UnsupportedVersionException and fall back to the old behavior.
> >
> > There are two main reasons for this:
> > 1. if we don't do this, we're changing the behavior for old clients in an
> > incompatible way. They used to get only unfenced brokers, now they get both
> > and they have no way of telling which is which.
> >
> > 2. new clients still would like to know whether they're getting
> > everything, or just unfenced brokers, for informational reasons.
> >
> > Probably the command-line tool could print out something like "fenced
> > brokers will not be shown" in the case where it's talking to an old server.
> > Or the tool could have a flag like --show-fenced which fails if it can't be
> > done?
> >
> > best,
> > Colin
> >
> >
> > On Fri, Sep 13, 2024, at 08:28, José Armando García Sancio wrote:
> > > Thanks Gantigmaa. See comments below. The KIP LGTM after this.
> > >
> > > On Thu, Sep 12, 2024 at 4:50 AM Gantigmaa Selenge <gs...@redhat.com>
> > wrote:
> > >> Will this make it more confusing for combined nodes? Users might expect
> > to
> > >> see both controller and broker under ROLES if it is a combined node.
> > Since
> > >> we would be able to output only one depending on the bootstrap option, I
> > >> wonder if it's better to stay consistent with the information in the
> > result
> > >> returned by the API.
> > >
> > > How about ENDPOINT-TYPE for the column name? That matches the keyword
> > > used in the request and better matches the semantic of the output.
> > >
> > > Also, I am okay if you want to leave it out. I thought the user may
> > > find it useful but if you find it confusing we can leave it out.
> > >
> > > Thanks,
> > > --
> > > -José
> >
> >
--
-José
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Gantigmaa Selenge <gs...@redhat.com>.
Hi everyone,
Thanks for reviewing the KIP again! I have updated it to address the latest
comments from Colin and Jose.
Going back to Jose's earlier comment here:
> Did you consider making "includeFencedBroker" field value implicitly
based on the DescribeClusterRequest version and the EndpointType? For
example, if the request version is 2 and the endpoint type is broker,
the response will include all of the brokers and if they are fenced or
unfenced. I am suggesting this because a request of endpointType
controller and includeFencedBroker true doesn't make sense. In
general, users of the AdminClient cannot easily discover the API
versions supported by the destination node since the destination node
may not be known when the admin call is made.
I think admins would request fenced brokers only in certain scenarios, so
perhaps it makes sense to include them only when requested via the new
option. If the endpoint type is controller and the option to include fenced
brokers set to true, the Admin Client can throw an
"IllegalArgumentException" without sending the request to the controller.
If you are happy with the updates I made, I would like to start the vote on
this KIP. Thank you.
Regards,
Gantigmaa
On Sat, Sep 14, 2024 at 9:51 PM Colin McCabe <cm...@apache.org> wrote:
> Hi Gantigmaa Selenge,
>
> I think we should have a boolean in the request like "includeFenced" which
> defaults to false. Then if we can't include the fenced brokers, we throw an
> UnsupportedVersionException and fall back to the old behavior.
>
> There are two main reasons for this:
> 1. if we don't do this, we're changing the behavior for old clients in an
> incompatible way. They used to get only unfenced brokers, now they get both
> and they have no way of telling which is which.
>
> 2. new clients still would like to know whether they're getting
> everything, or just unfenced brokers, for informational reasons.
>
> Probably the command-line tool could print out something like "fenced
> brokers will not be shown" in the case where it's talking to an old server.
> Or the tool could have a flag like --show-fenced which fails if it can't be
> done?
>
> best,
> Colin
>
>
> On Fri, Sep 13, 2024, at 08:28, José Armando García Sancio wrote:
> > Thanks Gantigmaa. See comments below. The KIP LGTM after this.
> >
> > On Thu, Sep 12, 2024 at 4:50 AM Gantigmaa Selenge <gs...@redhat.com>
> wrote:
> >> Will this make it more confusing for combined nodes? Users might expect
> to
> >> see both controller and broker under ROLES if it is a combined node.
> Since
> >> we would be able to output only one depending on the bootstrap option, I
> >> wonder if it's better to stay consistent with the information in the
> result
> >> returned by the API.
> >
> > How about ENDPOINT-TYPE for the column name? That matches the keyword
> > used in the request and better matches the semantic of the output.
> >
> > Also, I am okay if you want to leave it out. I thought the user may
> > find it useful but if you find it confusing we can leave it out.
> >
> > Thanks,
> > --
> > -José
>
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Colin McCabe <cm...@apache.org>.
Hi Gantigmaa Selenge,
I think we should have a boolean in the request like "includeFenced" which defaults to false. Then if we can't include the fenced brokers, we throw an UnsupportedVersionException and fall back to the old behavior.
There are two main reasons for this:
1. if we don't do this, we're changing the behavior for old clients in an incompatible way. They used to get only unfenced brokers, now they get both and they have no way of telling which is which.
2. new clients still would like to know whether they're getting everything, or just unfenced brokers, for informational reasons.
Probably the command-line tool could print out something like "fenced brokers will not be shown" in the case where it's talking to an old server. Or the tool could have a flag like --show-fenced which fails if it can't be done?
best,
Colin
On Fri, Sep 13, 2024, at 08:28, José Armando García Sancio wrote:
> Thanks Gantigmaa. See comments below. The KIP LGTM after this.
>
> On Thu, Sep 12, 2024 at 4:50 AM Gantigmaa Selenge <gs...@redhat.com> wrote:
>> Will this make it more confusing for combined nodes? Users might expect to
>> see both controller and broker under ROLES if it is a combined node. Since
>> we would be able to output only one depending on the bootstrap option, I
>> wonder if it's better to stay consistent with the information in the result
>> returned by the API.
>
> How about ENDPOINT-TYPE for the column name? That matches the keyword
> used in the request and better matches the semantic of the output.
>
> Also, I am okay if you want to leave it out. I thought the user may
> find it useful but if you find it confusing we can leave it out.
>
> Thanks,
> --
> -José
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by José Armando García Sancio <js...@confluent.io.INVALID>.
Thanks Gantigmaa. See comments below. The KIP LGTM after this.
On Thu, Sep 12, 2024 at 4:50 AM Gantigmaa Selenge <gs...@redhat.com> wrote:
> Will this make it more confusing for combined nodes? Users might expect to
> see both controller and broker under ROLES if it is a combined node. Since
> we would be able to output only one depending on the bootstrap option, I
> wonder if it's better to stay consistent with the information in the result
> returned by the API.
How about ENDPOINT-TYPE for the column name? That matches the keyword
used in the request and better matches the semantic of the output.
Also, I am okay if you want to leave it out. I thought the user may
find it useful but if you find it confusing we can leave it out.
Thanks,
--
-José
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Gantigmaa Selenge <gs...@redhat.com>.
Thank you Jose.
> I think that we should always output the RACK column even if all of
the nodes do not contain rack information. This would make it easier
for Kafka operators that develop scripts which use and parse CLI
outputs. What do you think?
That makes sense to me, so I updated the KIP and the CLI output will always
include the rack information.
> Sounds good to me. Should we also add a ROLES column to the output
where the possible values are "broker" and "controller"?
--bootstrap-server will always set that column to "broker" while
--bootstrap-controller will always set that column to "controller".
This would reinforce to users that are less familiar with the
implementation of the role of the returned endpoints.
Will this make it more confusing for combined nodes? Users might expect to
see both controller and broker under ROLES if it is a combined node. Since
we would be able to output only one depending on the bootstrap option, I
wonder if it's better to stay consistent with the information in the result
returned by the API.
> While thinking about the comment above, I realized that something
important to note is that this RPC and command return the node
_endpoints_ and not necessarily the nodes. For example, what happens
if some of the brokers (1 and 2) have the advertised listeners (A, B
and C) and some of the brokers (3 and 4) have the advertised listeners
(A, B) yet the user uses a bootstrap server endpoint that maps to the
listener C? The current implementation would only return brokers 1 and
2 since those are the only brokers that have listener C.
Are you planning to keep this implementation? I think we have to, to
keep it compatible with the previous implementation. If so, maybe we
can make this clearer by calling the command "list-endpoints" instead
of "list-nodes". What do you think?
This is a good point. It seems like nodes are retrieved from either
BrokerRegistration or ControllerRegistration but then they are filtered
based on the listener used for the bootstrap. I would have expected the
describeCluster API to return all the nodes that are registered, not just
the ones matching the listener because the result contains Nodes, rather
than endpoints. However, I'm not sure this implementation should change
with this KIP. So I will change the new command to list-endpoints from
list-nodes to make it clearer with the CLI at least.
Regards,
Gantigmaa
On Tue, Sep 10, 2024 at 4:18 PM José Armando García Sancio
<js...@confluent.io.invalid> wrote:
> Gantigmaa, thanks for the changes, they look good to me in general, I
> have some UX questions.
>
> The KIP mentions the following:
> > Also if there is no rack information for any of the nodes, the RACK
> column will be omitted from the output.
>
> What will the tool output for nodes that don't have a rack when other
> nodes contain rack information?
>
> I think that we should always output the RACK column even if all of
> the nodes do not contain rack information. This would make it easier
> for Kafka operators that develop scripts which use and parse CLI
> outputs. What do you think?
>
> > I think we can change the command to "list-nodes" and
> output either controller nodes or broker nodes depending on the bootstrap
> argument.
>
> Sounds good to me. Should we also add a ROLES column to the output
> where the possible values are "broker" and "controller"?
> --bootstrap-server will always set that column to "broker" while
> --bootstrap-controller will always set that column to "controller".
> This would reinforce to users that are less familiar with the
> implementation of the role of the returned endpoints.
>
> While thinking about the comment above, I realized that something
> important to note is that this RPC and command return the node
> _endpoints_ and not necessarily the nodes. For example, what happens
> if some of the brokers (1 and 2) have the advertised listeners (A, B
> and C) and some of the brokers (3 and 4) have the advertised listeners
> (A, B) yet the user uses a bootstrap server endpoint that maps to the
> listener C? The current implementation would only return brokers 1 and
> 2 since those are the only brokers that have listener C.
>
> Are you planning to keep this implementation? I think we have to, to
> keep it compatible with the previous implementation. If so, maybe we
> can make this clearer by calling the command "list-endpoints" instead
> of "list-nodes". What do you think?
>
> Thanks
> --
> -José
>
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by José Armando García Sancio <js...@confluent.io.INVALID>.
Gantigmaa, thanks for the changes, they look good to me in general, I
have some UX questions.
The KIP mentions the following:
> Also if there is no rack information for any of the nodes, the RACK column will be omitted from the output.
What will the tool output for nodes that don't have a rack when other
nodes contain rack information?
I think that we should always output the RACK column even if all of
the nodes do not contain rack information. This would make it easier
for Kafka operators that develop scripts which use and parse CLI
outputs. What do you think?
> I think we can change the command to "list-nodes" and
output either controller nodes or broker nodes depending on the bootstrap
argument.
Sounds good to me. Should we also add a ROLES column to the output
where the possible values are "broker" and "controller"?
--bootstrap-server will always set that column to "broker" while
--bootstrap-controller will always set that column to "controller".
This would reinforce to users that are less familiar with the
implementation of the role of the returned endpoints.
While thinking about the comment above, I realized that something
important to note is that this RPC and command return the node
_endpoints_ and not necessarily the nodes. For example, what happens
if some of the brokers (1 and 2) have the advertised listeners (A, B
and C) and some of the brokers (3 and 4) have the advertised listeners
(A, B) yet the user uses a bootstrap server endpoint that maps to the
listener C? The current implementation would only return brokers 1 and
2 since those are the only brokers that have listener C.
Are you planning to keep this implementation? I think we have to, to
keep it compatible with the previous implementation. If so, maybe we
can make this clearer by calling the command "list-endpoints" instead
of "list-nodes". What do you think?
Thanks
--
-José
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Gantigmaa Selenge <gs...@redhat.com>.
Thank you Luke for taking a look again.
If there are no further comments, I would like to start the vote for this
KIP tomorrow.
Regards,
Gantigmaa
On Thu, Sep 5, 2024 at 8:30 AM Luke Chen <sh...@gmail.com> wrote:
> Hi Tina,
>
> Thanks for updating the KIP.
> This change makes sense to me.
>
> Thanks.
> Luke
>
> On Wed, Aug 28, 2024 at 6:34 PM Gantigmaa Selenge <gs...@redhat.com>
> wrote:
>
> > Thank you both for reviewing the KIP again.
> >
> > Addressing Colin's comments:
> >
> > > The admin client changes don't seem to include a way to find out if a
> > given broker is fenced, by examining the result. Seems like we need that
> as
> > well?
> >
> > I think admins usually send describeCluster requests to find out about
> > cluster's node ids and their descriptions. So I'm not sure if we really
> > need the ability to describe a specific node as they would still need to
> > send a request to find out the node id first, which could already include
> > the fenced state. However, if you think it's useful to only describe a
> > particular node, we can add that to the admin client changes. I guess we
> > would need to add another field to the DescribeClusterOptions to specify
> a
> > node id so that the response returns the description for that node only
> > (ignoring rest of the nodes in the result returned from broker)?
> >
> > > You need to describe what happens when the client requests to see
> fenced
> > brokers, but the protocol doesn't support that. I would suggest
> > UnsupportedVersionException.
> >
> > With Jose's suggestion to remove the includeFencedBroker option, we might
> > not need this exception anymore. Admin clients connecting to older
> brokers
> > would simply not receive the fenced state of the nodes.
> >
> > Addressing Jose's comments:
> >
> > > Did you consider making "includeFencedBroker" field value implicitly
> > based on the DescribeClusterRequest version and the EndpointType? For
> > example, if the request version is 2 and the endpoint type is broker,
> > the response will include all of the brokers and if they are fenced or
> > unfenced. I am suggesting this because a request of endpointType
> > controller and includeFencedBroker true doesn't make sense. In
> > general, users of the AdminClient cannot easily discover the API
> > versions supported by the destination node since the destination node
> > may not be known when the admin call is made.
> >
> > This is a good point! I agree that endpointType controller and
> > includeFencedBroker true doesn't make sense. I thought the
> > includeFencedBroker option would be ignored since there is no fenced
> state
> > in the ControllerRegistration. So I think what you are suggesting is that
> > instead of having the includeFencedBroker option in the request, we would
> > return all broker nodes and their description including the fenced state,
> > if the request version is 2. I guess if endpointType is controller,
> > controller nodes will be returned with a fenced state set to false (it
> is a
> > boolean field in Node class). I'm happy to go with this approach because
> I
> > think it is even simpler.
> >
> >
> > > It is unfortunate that this RPC is handled using the metadata cache.
> > That means that the user cannot find out when was the last time the
> > broker contacted the controller cluster. Knowing that a broker is
> > fenced is not enough information to determine to unregister a broker.
> > If the user knows that the broker has been fenced for hours or day, it
> > may be enough information to decide to unregister the cluster.
> >
> > I understand that we can't determine how long a broker node has been in a
> > fenced state with the API. The motivation of this proposal is to help
> > admins to find out which broker is fenced, when they are performing scale
> > down operation. Nodes wouldn't usually get unregistered unless the
> > cluster is being scaled down, however admin could also find out the id
> of a
> > fenced broker using the DescribeCluster API and use it to check the
> metrics
> > such as lastFetchTimeStamp to determine how longs it's been fenced.
> >
> > > For this command to work, the user must pass --bootstrap-server. If
> > the user passes --bootstrap-controller, the controller nodes will get
> > listed instead. Are you planning to also implement "kafka-cluster
> > --bootstrap-controller <endpoints> list-controllers"? The other option
> > is to make the command "list-nodes" and it will list the controllers
> > or brokers based on the bootstrap server used. Or are you planning to
> > fix the controller side and broker side implementation so that
> > controller nodes can return registered brokers and broker nodes can
> > return registered controllers?
> >
> > I was planning to limit the "list-brokers" command only to be used with
> > --bootstrap-server. If the command was used with --bootstrap-controller,
> > the user would get an error back. I should have made it clear in the KIP.
> > However, thinking about this now, if we go ahead with the suggestion from
> > your first point, I think we can change the command to "list-nodes" and
> > output either controller nodes or broker nodes depending on the bootstrap
> > argument. The output will have a fenced state printed only if
> > --bootstrap-server is used. What do you think?
> >
> > > Is the tool going to print the string "→ (Broker 2 has shutdown but
> > still registered)" to stdout?
> >
> > Sorry, that was meant to be just a comment clarifying the new row, which
> > may not be necessary. I will remove it.
> >
> > Regards,
> > Gantigmaa
> >
> > On Mon, Aug 26, 2024 at 3:19 PM José Armando García Sancio
> > <js...@confluent.io.invalid> wrote:
> >
> > > Thanks for the KIP Gantigmaa,
> > >
> > > > { "name": "IncludeFencedBrokers", "type": "bool", "versions": "2+",
> > > "about": "Whether to include fenced brokers." }
> > >
> > > Did you consider making "includeFencedBroker" field value implicitly
> > > based on the DescribeClusterRequest version and the EndpointType? For
> > > example, if the request version is 2 and the endpoint type is broker,
> > > the response will include all of the brokers and if they are fenced or
> > > unfenced. I am suggesting this because a request of endpointType
> > > controller and includeFencedBroker true doesn't make sense. In
> > > general, users of the AdminClient cannot easily discover the API
> > > versions supported by the destination node since the destination node
> > > may not be known when the admin call is made.
> > >
> > > > { "name": "Fenced", "type": "bool", "versions": "2+", "about":
> "Whether
> > > the broker is fenced." }
> > >
> > > It is unfortunate that this RPC is handled using the metadata cache.
> > > That means that the user cannot find out when was the last time the
> > > broker contacted the controller cluster. Knowing that a broker is
> > > fenced is not enough information to determine to unregister a broker.
> > > If the user knows that the broker has been fenced for hours or day, it
> > > may be enough information to decide to unregister the cluster.
> > >
> > > > kafka-cluster.sh --bootstrap-server localhost:9092 list-brokers
> > >
> > > For this command to work, the user must pass --bootstrap-server. If
> > > the user passes --bootstrap-controller, the controller nodes will get
> > > listed instead. Are you planning to also implement "kafka-cluster
> > > --bootstrap-controller <endpoints> list-controllers"? The other option
> > > is to make the command "list-nodes" and it will list the controllers
> > > or brokers based on the bootstrap server used. Or are you planning to
> > > fix the controller side and broker side implementation so that
> > > controller nodes can return registered brokers and broker nodes can
> > > return registered controllers?
> > >
> > > I guess the other option is to limit the kafka-cluster tool for
> > > brokers and they must use the kafka-metadata-quorum tool for
> > > controllers. What do you think?
> > >
> > > > 2 broker-2 9092 fenced → (Broker 2 has shutdown but
> > > still registered)
> > >
> > > Is the tool going to print the string "→ (Broker 2 has shutdown but
> > > still registered)" to stdout?
> > >
> > > Thanks,
> > > -José
> > >
> > >
> >
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Luke Chen <sh...@gmail.com>.
Hi Tina,
Thanks for updating the KIP.
This change makes sense to me.
Thanks.
Luke
On Wed, Aug 28, 2024 at 6:34 PM Gantigmaa Selenge <gs...@redhat.com>
wrote:
> Thank you both for reviewing the KIP again.
>
> Addressing Colin's comments:
>
> > The admin client changes don't seem to include a way to find out if a
> given broker is fenced, by examining the result. Seems like we need that as
> well?
>
> I think admins usually send describeCluster requests to find out about
> cluster's node ids and their descriptions. So I'm not sure if we really
> need the ability to describe a specific node as they would still need to
> send a request to find out the node id first, which could already include
> the fenced state. However, if you think it's useful to only describe a
> particular node, we can add that to the admin client changes. I guess we
> would need to add another field to the DescribeClusterOptions to specify a
> node id so that the response returns the description for that node only
> (ignoring rest of the nodes in the result returned from broker)?
>
> > You need to describe what happens when the client requests to see fenced
> brokers, but the protocol doesn't support that. I would suggest
> UnsupportedVersionException.
>
> With Jose's suggestion to remove the includeFencedBroker option, we might
> not need this exception anymore. Admin clients connecting to older brokers
> would simply not receive the fenced state of the nodes.
>
> Addressing Jose's comments:
>
> > Did you consider making "includeFencedBroker" field value implicitly
> based on the DescribeClusterRequest version and the EndpointType? For
> example, if the request version is 2 and the endpoint type is broker,
> the response will include all of the brokers and if they are fenced or
> unfenced. I am suggesting this because a request of endpointType
> controller and includeFencedBroker true doesn't make sense. In
> general, users of the AdminClient cannot easily discover the API
> versions supported by the destination node since the destination node
> may not be known when the admin call is made.
>
> This is a good point! I agree that endpointType controller and
> includeFencedBroker true doesn't make sense. I thought the
> includeFencedBroker option would be ignored since there is no fenced state
> in the ControllerRegistration. So I think what you are suggesting is that
> instead of having the includeFencedBroker option in the request, we would
> return all broker nodes and their description including the fenced state,
> if the request version is 2. I guess if endpointType is controller,
> controller nodes will be returned with a fenced state set to false (it is a
> boolean field in Node class). I'm happy to go with this approach because I
> think it is even simpler.
>
>
> > It is unfortunate that this RPC is handled using the metadata cache.
> That means that the user cannot find out when was the last time the
> broker contacted the controller cluster. Knowing that a broker is
> fenced is not enough information to determine to unregister a broker.
> If the user knows that the broker has been fenced for hours or day, it
> may be enough information to decide to unregister the cluster.
>
> I understand that we can't determine how long a broker node has been in a
> fenced state with the API. The motivation of this proposal is to help
> admins to find out which broker is fenced, when they are performing scale
> down operation. Nodes wouldn't usually get unregistered unless the
> cluster is being scaled down, however admin could also find out the id of a
> fenced broker using the DescribeCluster API and use it to check the metrics
> such as lastFetchTimeStamp to determine how longs it's been fenced.
>
> > For this command to work, the user must pass --bootstrap-server. If
> the user passes --bootstrap-controller, the controller nodes will get
> listed instead. Are you planning to also implement "kafka-cluster
> --bootstrap-controller <endpoints> list-controllers"? The other option
> is to make the command "list-nodes" and it will list the controllers
> or brokers based on the bootstrap server used. Or are you planning to
> fix the controller side and broker side implementation so that
> controller nodes can return registered brokers and broker nodes can
> return registered controllers?
>
> I was planning to limit the "list-brokers" command only to be used with
> --bootstrap-server. If the command was used with --bootstrap-controller,
> the user would get an error back. I should have made it clear in the KIP.
> However, thinking about this now, if we go ahead with the suggestion from
> your first point, I think we can change the command to "list-nodes" and
> output either controller nodes or broker nodes depending on the bootstrap
> argument. The output will have a fenced state printed only if
> --bootstrap-server is used. What do you think?
>
> > Is the tool going to print the string "→ (Broker 2 has shutdown but
> still registered)" to stdout?
>
> Sorry, that was meant to be just a comment clarifying the new row, which
> may not be necessary. I will remove it.
>
> Regards,
> Gantigmaa
>
> On Mon, Aug 26, 2024 at 3:19 PM José Armando García Sancio
> <js...@confluent.io.invalid> wrote:
>
> > Thanks for the KIP Gantigmaa,
> >
> > > { "name": "IncludeFencedBrokers", "type": "bool", "versions": "2+",
> > "about": "Whether to include fenced brokers." }
> >
> > Did you consider making "includeFencedBroker" field value implicitly
> > based on the DescribeClusterRequest version and the EndpointType? For
> > example, if the request version is 2 and the endpoint type is broker,
> > the response will include all of the brokers and if they are fenced or
> > unfenced. I am suggesting this because a request of endpointType
> > controller and includeFencedBroker true doesn't make sense. In
> > general, users of the AdminClient cannot easily discover the API
> > versions supported by the destination node since the destination node
> > may not be known when the admin call is made.
> >
> > > { "name": "Fenced", "type": "bool", "versions": "2+", "about": "Whether
> > the broker is fenced." }
> >
> > It is unfortunate that this RPC is handled using the metadata cache.
> > That means that the user cannot find out when was the last time the
> > broker contacted the controller cluster. Knowing that a broker is
> > fenced is not enough information to determine to unregister a broker.
> > If the user knows that the broker has been fenced for hours or day, it
> > may be enough information to decide to unregister the cluster.
> >
> > > kafka-cluster.sh --bootstrap-server localhost:9092 list-brokers
> >
> > For this command to work, the user must pass --bootstrap-server. If
> > the user passes --bootstrap-controller, the controller nodes will get
> > listed instead. Are you planning to also implement "kafka-cluster
> > --bootstrap-controller <endpoints> list-controllers"? The other option
> > is to make the command "list-nodes" and it will list the controllers
> > or brokers based on the bootstrap server used. Or are you planning to
> > fix the controller side and broker side implementation so that
> > controller nodes can return registered brokers and broker nodes can
> > return registered controllers?
> >
> > I guess the other option is to limit the kafka-cluster tool for
> > brokers and they must use the kafka-metadata-quorum tool for
> > controllers. What do you think?
> >
> > > 2 broker-2 9092 fenced → (Broker 2 has shutdown but
> > still registered)
> >
> > Is the tool going to print the string "→ (Broker 2 has shutdown but
> > still registered)" to stdout?
> >
> > Thanks,
> > -José
> >
> >
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Gantigmaa Selenge <gs...@redhat.com>.
Thank you both for reviewing the KIP again.
Addressing Colin's comments:
> The admin client changes don't seem to include a way to find out if a
given broker is fenced, by examining the result. Seems like we need that as
well?
I think admins usually send describeCluster requests to find out about
cluster's node ids and their descriptions. So I'm not sure if we really
need the ability to describe a specific node as they would still need to
send a request to find out the node id first, which could already include
the fenced state. However, if you think it's useful to only describe a
particular node, we can add that to the admin client changes. I guess we
would need to add another field to the DescribeClusterOptions to specify a
node id so that the response returns the description for that node only
(ignoring rest of the nodes in the result returned from broker)?
> You need to describe what happens when the client requests to see fenced
brokers, but the protocol doesn't support that. I would suggest
UnsupportedVersionException.
With Jose's suggestion to remove the includeFencedBroker option, we might
not need this exception anymore. Admin clients connecting to older brokers
would simply not receive the fenced state of the nodes.
Addressing Jose's comments:
> Did you consider making "includeFencedBroker" field value implicitly
based on the DescribeClusterRequest version and the EndpointType? For
example, if the request version is 2 and the endpoint type is broker,
the response will include all of the brokers and if they are fenced or
unfenced. I am suggesting this because a request of endpointType
controller and includeFencedBroker true doesn't make sense. In
general, users of the AdminClient cannot easily discover the API
versions supported by the destination node since the destination node
may not be known when the admin call is made.
This is a good point! I agree that endpointType controller and
includeFencedBroker true doesn't make sense. I thought the
includeFencedBroker option would be ignored since there is no fenced state
in the ControllerRegistration. So I think what you are suggesting is that
instead of having the includeFencedBroker option in the request, we would
return all broker nodes and their description including the fenced state,
if the request version is 2. I guess if endpointType is controller,
controller nodes will be returned with a fenced state set to false (it is a
boolean field in Node class). I'm happy to go with this approach because I
think it is even simpler.
> It is unfortunate that this RPC is handled using the metadata cache.
That means that the user cannot find out when was the last time the
broker contacted the controller cluster. Knowing that a broker is
fenced is not enough information to determine to unregister a broker.
If the user knows that the broker has been fenced for hours or day, it
may be enough information to decide to unregister the cluster.
I understand that we can't determine how long a broker node has been in a
fenced state with the API. The motivation of this proposal is to help
admins to find out which broker is fenced, when they are performing scale
down operation. Nodes wouldn't usually get unregistered unless the
cluster is being scaled down, however admin could also find out the id of a
fenced broker using the DescribeCluster API and use it to check the metrics
such as lastFetchTimeStamp to determine how longs it's been fenced.
> For this command to work, the user must pass --bootstrap-server. If
the user passes --bootstrap-controller, the controller nodes will get
listed instead. Are you planning to also implement "kafka-cluster
--bootstrap-controller <endpoints> list-controllers"? The other option
is to make the command "list-nodes" and it will list the controllers
or brokers based on the bootstrap server used. Or are you planning to
fix the controller side and broker side implementation so that
controller nodes can return registered brokers and broker nodes can
return registered controllers?
I was planning to limit the "list-brokers" command only to be used with
--bootstrap-server. If the command was used with --bootstrap-controller,
the user would get an error back. I should have made it clear in the KIP.
However, thinking about this now, if we go ahead with the suggestion from
your first point, I think we can change the command to "list-nodes" and
output either controller nodes or broker nodes depending on the bootstrap
argument. The output will have a fenced state printed only if
--bootstrap-server is used. What do you think?
> Is the tool going to print the string "→ (Broker 2 has shutdown but
still registered)" to stdout?
Sorry, that was meant to be just a comment clarifying the new row, which
may not be necessary. I will remove it.
Regards,
Gantigmaa
On Mon, Aug 26, 2024 at 3:19 PM José Armando García Sancio
<js...@confluent.io.invalid> wrote:
> Thanks for the KIP Gantigmaa,
>
> > { "name": "IncludeFencedBrokers", "type": "bool", "versions": "2+",
> "about": "Whether to include fenced brokers." }
>
> Did you consider making "includeFencedBroker" field value implicitly
> based on the DescribeClusterRequest version and the EndpointType? For
> example, if the request version is 2 and the endpoint type is broker,
> the response will include all of the brokers and if they are fenced or
> unfenced. I am suggesting this because a request of endpointType
> controller and includeFencedBroker true doesn't make sense. In
> general, users of the AdminClient cannot easily discover the API
> versions supported by the destination node since the destination node
> may not be known when the admin call is made.
>
> > { "name": "Fenced", "type": "bool", "versions": "2+", "about": "Whether
> the broker is fenced." }
>
> It is unfortunate that this RPC is handled using the metadata cache.
> That means that the user cannot find out when was the last time the
> broker contacted the controller cluster. Knowing that a broker is
> fenced is not enough information to determine to unregister a broker.
> If the user knows that the broker has been fenced for hours or day, it
> may be enough information to decide to unregister the cluster.
>
> > kafka-cluster.sh --bootstrap-server localhost:9092 list-brokers
>
> For this command to work, the user must pass --bootstrap-server. If
> the user passes --bootstrap-controller, the controller nodes will get
> listed instead. Are you planning to also implement "kafka-cluster
> --bootstrap-controller <endpoints> list-controllers"? The other option
> is to make the command "list-nodes" and it will list the controllers
> or brokers based on the bootstrap server used. Or are you planning to
> fix the controller side and broker side implementation so that
> controller nodes can return registered brokers and broker nodes can
> return registered controllers?
>
> I guess the other option is to limit the kafka-cluster tool for
> brokers and they must use the kafka-metadata-quorum tool for
> controllers. What do you think?
>
> > 2 broker-2 9092 fenced → (Broker 2 has shutdown but
> still registered)
>
> Is the tool going to print the string "→ (Broker 2 has shutdown but
> still registered)" to stdout?
>
> Thanks,
> -José
>
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by José Armando García Sancio <js...@confluent.io.INVALID>.
Thanks for the KIP Gantigmaa,
> { "name": "IncludeFencedBrokers", "type": "bool", "versions": "2+", "about": "Whether to include fenced brokers." }
Did you consider making "includeFencedBroker" field value implicitly
based on the DescribeClusterRequest version and the EndpointType? For
example, if the request version is 2 and the endpoint type is broker,
the response will include all of the brokers and if they are fenced or
unfenced. I am suggesting this because a request of endpointType
controller and includeFencedBroker true doesn't make sense. In
general, users of the AdminClient cannot easily discover the API
versions supported by the destination node since the destination node
may not be known when the admin call is made.
> { "name": "Fenced", "type": "bool", "versions": "2+", "about": "Whether the broker is fenced." }
It is unfortunate that this RPC is handled using the metadata cache.
That means that the user cannot find out when was the last time the
broker contacted the controller cluster. Knowing that a broker is
fenced is not enough information to determine to unregister a broker.
If the user knows that the broker has been fenced for hours or day, it
may be enough information to decide to unregister the cluster.
> kafka-cluster.sh --bootstrap-server localhost:9092 list-brokers
For this command to work, the user must pass --bootstrap-server. If
the user passes --bootstrap-controller, the controller nodes will get
listed instead. Are you planning to also implement "kafka-cluster
--bootstrap-controller <endpoints> list-controllers"? The other option
is to make the command "list-nodes" and it will list the controllers
or brokers based on the bootstrap server used. Or are you planning to
fix the controller side and broker side implementation so that
controller nodes can return registered brokers and broker nodes can
return registered controllers?
I guess the other option is to limit the kafka-cluster tool for
brokers and they must use the kafka-metadata-quorum tool for
controllers. What do you think?
> 2 broker-2 9092 fenced → (Broker 2 has shutdown but still registered)
Is the tool going to print the string "→ (Broker 2 has shutdown but
still registered)" to stdout?
Thanks,
-José
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Gantigmaa Selenge <gs...@redhat.com>.
Hi everyone,
I have updated the proposed solution of the KIP. Instead of extending the
DescribeQuorum API, it now suggests updating the DescribeCluster API to
optionally return fenced brokers. This should address the comments from
Colin and Jose.
Please let me know if you have any questions.
Regards,
Gantigmaa
On Thu, Aug 15, 2024 at 9:00 AM Gantigmaa Selenge <gs...@redhat.com>
wrote:
> Hi Colin and Jose,
>
> Thanks for taking a look at the KIP and providing feedback.
>
> I overlooked that the memory state containing observers will be lost when
> a controller restarts. You also made a good point about other observers
> that might be connecting to the quorum to read the current metadata. I
> agree that we should implement a way to list the fenced brokers using the
> DescribeClusterRequest instead of the DescribeQuorum API.
>
> I will update the KIP and get back to you all for further feedback. Thanks.
>
> Regards,
> Gantigmaa
>
> On Fri, Aug 9, 2024 at 11:48 PM José Armando García Sancio
> <js...@confluent.io.invalid> wrote:
>
>> Thanks for the KIP Gantigmaa!
>>
>> I agree with the motivation but it is not clear to me that this should be
>> solved in the KRaft layer. The KRaft leader only keeps track, in-memory,
>> of
>> observers that have fetched. It is possible, after a kraft leader change,
>> for this state to get lost. If the shutdown brokers never fetch they will
>> not show up in the kraft leader's observer list.
>>
>> As Colin has pointed out it is better to solve this in the metadata module
>> and the KRaft/Quorum controller. The QuorumController knows of all of the
>> registered brokers and their state: fence, unfence and even the heartbeat
>> state. Take a look at
>> org.apache.kafka.controller.BrokerHeartbeatManager.BrokerHeartbeatState
>> for
>> some of the heartbeat state tracked.
>>
>> To expose this information we can extend the kafka-cluster CLI, and also
>> extend Metadata RPC or introduce a new RPC that returns more broker level
>> information.
>>
>> Thanks,
>>
>> On Thu, Jul 25, 2024 at 9:21 AM Gantigmaa Selenge <gs...@redhat.com>
>> wrote:
>>
>> > Hi everyone,
>> >
>> > I would like to start a discussion on KIP-1073 that includes inactive
>> > observer nodes in the response for describeQuorum request.
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
>> >
>> > The initial discussion on this issue is here, if you would like to see
>> what
>> > was previously discussed:
>> > https://lists.apache.org/list.html?dev@kafka.apache.org
>> >
>> > Any feedback and suggestions for the KIP are welcome in this email
>> thread.
>> >
>> > Thank you.
>> > Regards,
>> > Gantigmaa Selenge
>> >
>>
>>
>> --
>> -José
>>
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Gantigmaa Selenge <gs...@redhat.com>.
Hi Colin and Jose,
Thanks for taking a look at the KIP and providing feedback.
I overlooked that the memory state containing observers will be lost when a
controller restarts. You also made a good point about other observers that
might be connecting to the quorum to read the current metadata. I agree
that we should implement a way to list the fenced brokers using the
DescribeClusterRequest instead of the DescribeQuorum API.
I will update the KIP and get back to you all for further feedback. Thanks.
Regards,
Gantigmaa
On Fri, Aug 9, 2024 at 11:48 PM José Armando García Sancio
<js...@confluent.io.invalid> wrote:
> Thanks for the KIP Gantigmaa!
>
> I agree with the motivation but it is not clear to me that this should be
> solved in the KRaft layer. The KRaft leader only keeps track, in-memory, of
> observers that have fetched. It is possible, after a kraft leader change,
> for this state to get lost. If the shutdown brokers never fetch they will
> not show up in the kraft leader's observer list.
>
> As Colin has pointed out it is better to solve this in the metadata module
> and the KRaft/Quorum controller. The QuorumController knows of all of the
> registered brokers and their state: fence, unfence and even the heartbeat
> state. Take a look at
> org.apache.kafka.controller.BrokerHeartbeatManager.BrokerHeartbeatState for
> some of the heartbeat state tracked.
>
> To expose this information we can extend the kafka-cluster CLI, and also
> extend Metadata RPC or introduce a new RPC that returns more broker level
> information.
>
> Thanks,
>
> On Thu, Jul 25, 2024 at 9:21 AM Gantigmaa Selenge <gs...@redhat.com>
> wrote:
>
> > Hi everyone,
> >
> > I would like to start a discussion on KIP-1073 that includes inactive
> > observer nodes in the response for describeQuorum request.
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
> >
> > The initial discussion on this issue is here, if you would like to see
> what
> > was previously discussed:
> > https://lists.apache.org/list.html?dev@kafka.apache.org
> >
> > Any feedback and suggestions for the KIP are welcome in this email
> thread.
> >
> > Thank you.
> > Regards,
> > Gantigmaa Selenge
> >
>
>
> --
> -José
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by José Armando García Sancio <js...@confluent.io.INVALID>.
Thanks for the KIP Gantigmaa!
I agree with the motivation but it is not clear to me that this should be
solved in the KRaft layer. The KRaft leader only keeps track, in-memory, of
observers that have fetched. It is possible, after a kraft leader change,
for this state to get lost. If the shutdown brokers never fetch they will
not show up in the kraft leader's observer list.
As Colin has pointed out it is better to solve this in the metadata module
and the KRaft/Quorum controller. The QuorumController knows of all of the
registered brokers and their state: fence, unfence and even the heartbeat
state. Take a look at
org.apache.kafka.controller.BrokerHeartbeatManager.BrokerHeartbeatState for
some of the heartbeat state tracked.
To expose this information we can extend the kafka-cluster CLI, and also
extend Metadata RPC or introduce a new RPC that returns more broker level
information.
Thanks,
On Thu, Jul 25, 2024 at 9:21 AM Gantigmaa Selenge <gs...@redhat.com>
wrote:
> Hi everyone,
>
> I would like to start a discussion on KIP-1073 that includes inactive
> observer nodes in the response for describeQuorum request.
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
>
> The initial discussion on this issue is here, if you would like to see what
> was previously discussed:
> https://lists.apache.org/list.html?dev@kafka.apache.org
>
> Any feedback and suggestions for the KIP are welcome in this email thread.
>
> Thank you.
> Regards,
> Gantigmaa Selenge
>
--
-José
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Colin McCabe <cm...@apache.org>.
On Fri, Aug 9, 2024, at 14:46, Colin McCabe wrote:
> Hi Gantigmaa,
>
> I agree with you that we need a way to list fenced brokers.
>
> However, I don't think this KIP is the right way to do that. This KIP,
> if I understand correctly, is about listing all Raft observers. The
> broker is one example of a Raft observer, but there may be others. For
> example, the metadata shell is supposed to be able to connect to the
> quorum and read the current metadata. (I think this feature was
> disabled for now because we didn't have enough time to finish it, but
> it's in the design.)
>
> Even worse, if the fenced broker is down (not runninng), it will not
> show up as a Raft observer at all. Therefore, you won't be able to do
> what you want with this KIP in its current form.
Actually, to drill down on this a bit: I think inactive observers might show up in some scenarios, but not all. Like if you roll the controller cluster, that in-memory state will probably get lost. In any case, my point remains: this isn't the right way.
best,
Colin
>
> A better idea would be to add a flag to DescribeClusterRequest that
> allowed us to list fenced brokers. You would also want to add a
> corresponding isFenced field to each broker's information in
> DescribeClusterResponse.
>
> Then you would probably add something like a list-brokers action to the
> kafka-cluster.sh command line. It makes more sense to put it there
> anyway, since that's where unregister is.
>
> best,
> Colin
>
>
> On the thread you linked, you pointed
>
> On Thu, Aug 8, 2024, at 09:56, Gantigmaa Selenge wrote:
>> Thank you Luke for taking a look at the KIP.
>>
>> I have corrected the typo and taken the suggestion of including replication
>> states for inactive observers when "–include-inactive-observers" option is
>> used with "
>> kafka-metadata-quorum.sh describe --replication", as this might be useful
>> for finding out how long an observer node has been inactive.
>>
>> Regards,
>> Gantigmaa
>>
>> On Thu, Aug 8, 2024 at 9:31 AM Luke Chen <sh...@gmail.com> wrote:
>>
>>> Hi Gantigmaa,
>>>
>>> Thanks for the KIP!
>>> The motivation and change looks good to me.
>>>
>>> Some comments:
>>> 1. typo: When a KRaft broker node shuts down, it is in "fenced" state, not
>>> "unfenced" state
>>> 2. Will the "–include-inactive-observers" option apply to "
>>> kafka-metadata-quorum.sh describe --replication"?
>>> I don't think we must have it, but maybe it's useful to let users know the
>>> offset lag for the inactive observers?
>>>
>>> Thank you.
>>> Luke
>>>
>>> On Thu, Jul 25, 2024 at 9:21 PM Gantigmaa Selenge <gs...@redhat.com>
>>> wrote:
>>>
>>> > Hi everyone,
>>> >
>>> > I would like to start a discussion on KIP-1073 that includes inactive
>>> > observer nodes in the response for describeQuorum request.
>>> >
>>> >
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
>>> >
>>> > The initial discussion on this issue is here, if you would like to see
>>> what
>>> > was previously discussed:
>>> > https://lists.apache.org/list.html?dev@kafka.apache.org
>>> >
>>> > Any feedback and suggestions for the KIP are welcome in this email
>>> thread.
>>> >
>>> > Thank you.
>>> > Regards,
>>> > Gantigmaa Selenge
>>> >
>>>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Colin McCabe <cm...@apache.org>.
Hi Gantigmaa,
I agree with you that we need a way to list fenced brokers.
However, I don't think this KIP is the right way to do that. This KIP, if I understand correctly, is about listing all Raft observers. The broker is one example of a Raft observer, but there may be others. For example, the metadata shell is supposed to be able to connect to the quorum and read the current metadata. (I think this feature was disabled for now because we didn't have enough time to finish it, but it's in the design.)
Even worse, if the fenced broker is down (not runninng), it will not show up as a Raft observer at all. Therefore, you won't be able to do what you want with this KIP in its current form.
A better idea would be to add a flag to DescribeClusterRequest that allowed us to list fenced brokers. You would also want to add a corresponding isFenced field to each broker's information in DescribeClusterResponse.
Then you would probably add something like a list-brokers action to the kafka-cluster.sh command line. It makes more sense to put it there anyway, since that's where unregister is.
best,
Colin
On the thread you linked, you pointed
On Thu, Aug 8, 2024, at 09:56, Gantigmaa Selenge wrote:
> Thank you Luke for taking a look at the KIP.
>
> I have corrected the typo and taken the suggestion of including replication
> states for inactive observers when "–include-inactive-observers" option is
> used with "
> kafka-metadata-quorum.sh describe --replication", as this might be useful
> for finding out how long an observer node has been inactive.
>
> Regards,
> Gantigmaa
>
> On Thu, Aug 8, 2024 at 9:31 AM Luke Chen <sh...@gmail.com> wrote:
>
>> Hi Gantigmaa,
>>
>> Thanks for the KIP!
>> The motivation and change looks good to me.
>>
>> Some comments:
>> 1. typo: When a KRaft broker node shuts down, it is in "fenced" state, not
>> "unfenced" state
>> 2. Will the "–include-inactive-observers" option apply to "
>> kafka-metadata-quorum.sh describe --replication"?
>> I don't think we must have it, but maybe it's useful to let users know the
>> offset lag for the inactive observers?
>>
>> Thank you.
>> Luke
>>
>> On Thu, Jul 25, 2024 at 9:21 PM Gantigmaa Selenge <gs...@redhat.com>
>> wrote:
>>
>> > Hi everyone,
>> >
>> > I would like to start a discussion on KIP-1073 that includes inactive
>> > observer nodes in the response for describeQuorum request.
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
>> >
>> > The initial discussion on this issue is here, if you would like to see
>> what
>> > was previously discussed:
>> > https://lists.apache.org/list.html?dev@kafka.apache.org
>> >
>> > Any feedback and suggestions for the KIP are welcome in this email
>> thread.
>> >
>> > Thank you.
>> > Regards,
>> > Gantigmaa Selenge
>> >
>>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Gantigmaa Selenge <gs...@redhat.com>.
Thank you Luke for taking a look at the KIP.
I have corrected the typo and taken the suggestion of including replication
states for inactive observers when "–include-inactive-observers" option is
used with "
kafka-metadata-quorum.sh describe --replication", as this might be useful
for finding out how long an observer node has been inactive.
Regards,
Gantigmaa
On Thu, Aug 8, 2024 at 9:31 AM Luke Chen <sh...@gmail.com> wrote:
> Hi Gantigmaa,
>
> Thanks for the KIP!
> The motivation and change looks good to me.
>
> Some comments:
> 1. typo: When a KRaft broker node shuts down, it is in "fenced" state, not
> "unfenced" state
> 2. Will the "–include-inactive-observers" option apply to "
> kafka-metadata-quorum.sh describe --replication"?
> I don't think we must have it, but maybe it's useful to let users know the
> offset lag for the inactive observers?
>
> Thank you.
> Luke
>
> On Thu, Jul 25, 2024 at 9:21 PM Gantigmaa Selenge <gs...@redhat.com>
> wrote:
>
> > Hi everyone,
> >
> > I would like to start a discussion on KIP-1073 that includes inactive
> > observer nodes in the response for describeQuorum request.
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
> >
> > The initial discussion on this issue is here, if you would like to see
> what
> > was previously discussed:
> > https://lists.apache.org/list.html?dev@kafka.apache.org
> >
> > Any feedback and suggestions for the KIP are welcome in this email
> thread.
> >
> > Thank you.
> > Regards,
> > Gantigmaa Selenge
> >
>
Re: [DISCUSS] KIP-1073 Return inactive observer nodes in DescribeQuorum response
Posted by Luke Chen <sh...@gmail.com>.
Hi Gantigmaa,
Thanks for the KIP!
The motivation and change looks good to me.
Some comments:
1. typo: When a KRaft broker node shuts down, it is in "fenced" state, not
"unfenced" state
2. Will the "–include-inactive-observers" option apply to "
kafka-metadata-quorum.sh describe --replication"?
I don't think we must have it, but maybe it's useful to let users know the
offset lag for the inactive observers?
Thank you.
Luke
On Thu, Jul 25, 2024 at 9:21 PM Gantigmaa Selenge <gs...@redhat.com>
wrote:
> Hi everyone,
>
> I would like to start a discussion on KIP-1073 that includes inactive
> observer nodes in the response for describeQuorum request.
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1073%3A+Return+inactive+observer+nodes+in+DescribeQuorum+response
>
> The initial discussion on this issue is here, if you would like to see what
> was previously discussed:
> https://lists.apache.org/list.html?dev@kafka.apache.org
>
> Any feedback and suggestions for the KIP are welcome in this email thread.
>
> Thank you.
> Regards,
> Gantigmaa Selenge
>