Hi David, Thanks again for working on this. It's an important one, for sure. It looks like the proposal has changed a bit so I'll re-review.
With regard to the topic ID field in AlterPartitionRequest, I think this is a good idea. It's also good that it replaces the name instead of supplementing it, as well. However, I wonder if this will be feasible for ZK-based brokers to do. We still support pre-topic-id IBP versions there, right? This might end up being more complex than you were hoping. We should add a comment in AlterPartitionResponse about the new error code INELIGIBLE_REPLICA. This is very important for error codes so we can track which ones are returned in which RPC version. I also wonder if we should add a special error code for the case where the AlterPartitions call completes a reassignment AND the completed reassignment no longer includes the previous leader. We have been overloading FENCED_LEADER_EPOCH for this case, but this is coonfusing to operators (especially since this RPC does not support error messages, as opposed to codes) I was hoping to avoid adding an extra "inControlledShutdown" state, but the arguments here have convinced me. I agree we kind of need this to handle the case where we are shutting down everything (common in tests) and also to avoid extra churn in a few roll cases. One thing, though, is that we should define how this interacts with replica placement. It seems to me that replicas should not be able to be placed on these inControlledShutdown nodes (unless done manually via the explicit placement API). I also think we should spell out the fact that once you go into controlled shutdown, you don't come out except by creating a new broker instance. (new incarnation ID). This also makes me wonder if we need to support the shutting down -> not shutting down transition in BrokerRegistrationChangeRecord, since we don't plan on using that transition. Finally, RegisterBrokerRecord / BrokerRegistrationChangeRecord should be bumped to the next RPC version since we have added a new field. You will also need to assign yourself a new IBP / MetadataVersion. (For BrokerRegistrationChangeRecord it would be possible to avoid the version bump, since we're using tagged fields, but it's better to have it for consistency, I think.) best, Colin On Tue, May 31, 2022, at 08:36, David Jacot wrote: > Hi folks, > > I'd like to start a vote for KIP-841: > https://cwiki.apache.org/confluence/x/phmhD. > > Thanks, > David
