Thanks for the updates. The latest KIP LGTM.
On Fri, Sep 20, 2024 at 8:38 AM Gantigmaa Selenge 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 conside
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
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
Thanks Gantigmaa. See comments below. The KIP LGTM after this.
On Thu, Sep 12, 2024 at 4:50 AM Gantigmaa Selenge 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 o
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
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
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 wrote:
> Hi Tina,
>
> Thanks for updating the KIP.
> This change makes sense to me.
>
> Thanks.
> Luke
>
>
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 abo
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
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
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.
Reg
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 a
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
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 examp
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, th
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
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
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 issu
18 matches
Mail list logo
