Hi all, I'm thinking of calling a vote on KIP-631 on Monday. Let me know if there's any more comments I should address before I start the vote.
cheers, Colin On Tue, Aug 11, 2020, at 05:39, Unmesh Joshi wrote: > >>Hi Unmesh, > >>Thanks, I'll take a look. > Thanks. I will be adding more to the prototype and will be happy to help > and collaborate. > > Thanks, > Unmesh > > On Tue, Aug 11, 2020 at 12:28 AM Colin McCabe <cmcc...@apache.org> wrote: > > > Hi Jose, > > > > That'a s good point that I hadn't considered. It's probably worth having > > a separate leader change message, as you mentioned. > > > > Hi Unmesh, > > > > Thanks, I'll take a look. > > > > best, > > Colin > > > > > > On Fri, Aug 7, 2020, at 11:56, Jose Garcia Sancio wrote: > > > Hi Unmesh, > > > > > > Very cool prototype! > > > > > > Hi Colin, > > > > > > The KIP proposes a record called IsrChange which includes the > > > partition, topic, isr, leader and leader epoch. During normal > > > operation ISR changes do not result in leader changes. Similarly, > > > leader changes do not necessarily involve ISR changes. The controller > > > implementation that uses ZK modeled them together because > > > 1. All of this information is stored in one znode. > > > 2. ZK's optimistic lock requires that you specify the new value > > completely > > > 3. The change to that znode was being performed by both the controller > > > and the leader. > > > > > > None of these reasons are true in KIP-500. Have we considered having > > > two different records? For example > > > > > > 1. IsrChange record which includes topic, partition, isr > > > 2. LeaderChange record which includes topic, partition, leader and > > leader epoch. > > > > > > I suspect that making this change will also require changing the > > > message AlterIsrRequest introduced in KIP-497: Add inter-broker API to > > > alter ISR. > > > > > > Thanks > > > -Jose > > > > > >