Hi Mayank/José,

> On Jul 13, 2023, at 8:20 AM, José Armando García Sancio 
> <jsan...@confluent.io.INVALID> wrote:
> 
> Hi Mayank, thanks for the KIP. I look forward to this improvement for
> new clients.
> 
> Some comments below.
> 
> On Thu, Jul 13, 2023 at 7:15 AM Mayank Shekhar Narula
> <mayanks.nar...@gmail.com> wrote:
>> Following KIP is up for discussion. Thanks for your feedback
> 
> Regarding the FETCH response changes:
> 1. Tagged field 2 already exists. Looks like you can use tagged field 3.
> 
> 2. The CurrentLeaderBroker should not be within PartitionData. It is
> possible to have multiple partitions that would change leadership to
> the same broker/node. We should instead move that information to a
> top-level field that is an array of (replica id, host, port, rack).

Is the requested restructuring of the response “simply” to preserve bytes, or 
is it possible that the fetch response could/should/would return leadership 
changes for partitions that we’re specifically requested?

> 3. In the future, I may use this information in the KRaft/Metadata
> implementation of FETCH. In that implementation not all of the
> replicas are brokers.

Side point: any references to the change you’re referring to? The idea of 
non-brokers serving as replicas is blowing my mind a bit :) 

> Do you mind removing all references to the word
> broker in the description and field name. Maybe you can use the word
> replica instead. How about something like this:
>    { "name": "NodeEndpoints", "type": "[]NodeEndpoint", "versions":
> "15+", "taggedVersions": "15+", "tag": 3,
>      "about": "Endpoint information for all current leaders
> enumerated in PartitionData.", "fields": [
>          { "name": "ReplicaId", "type": "int32", "versions": "15+",
> "mapKey": true, "entityType": "brokerId",
>            "about": "The ID of the associated replica"},
>          { "name": "Host", "type": "string", "versions": "15+",
>            "about": "The replica's hostname." },
>          { "name": "Port", "type": "int32", "versions": "15+",
>            "about": "The replica's port." },
>          { "name": "Rack", "type": "string", "versions": "15+",
> "ignorable": true, "default": "null",
>            "about": "The rack of the replica, or null if it has not
> been assigned to a rack." }
>      ]},
> 
> Regarding the PRODUCE response changes:
> 4. Can we make similar changes as the ones mentioned in bullet points
> 2. and 3 above?.
> 
> 5. If you make the changes enumerated in bullet point 4., you'll
> probably want to change the tag so that NodeEpoint has tag 0 while
> CurrentLeader has tag 1.
> 
> Thanks!
> -- 
> -José

Thanks!
Kirk

Reply via email to