Hi Onur, Comments inline.
On Wed, May 3, 2017 at 6:54 PM, Onur Karaman <onurkaraman.apa...@gmail.com> wrote: > Regarding the ControllerState and the potential for overlap, I think it > depends on our definition of controller state. While KAFKA-5028 allows only > a single ControllerEvent to be processed at a time, it still allows > interleavings for long-lasting actions like partition reassignment and > topic deletion. For instance, a topic can get created while another topic > is undergoing partition reassignment. In that sense, there is overlap. > However, in the sense of the ControllerEvents being processed, there can be > no overlap. > Yes, the proposal is for the event that is currently being processed, so no overlap. I clarified in the KIP. > > I also think adding the QueueSize and QueueTimeMs metrics could be a > premature move. I completely agree that these metrics would be valuable > given KAFKA-5028. However, I'm not sure whether the controller event queue > and controller thread as implemented today is actually here to stay or if > it's merely a first step in the controller redesign. Especially when > considering the possibility of moving away from the simple synchronous > zookeeper apis and having better control over handling zookeeper > disconnects and session expirations, it's possible that the queue and > thread could actually get ripped out of the controller and being part of > something more general, making these two controller-level metrics invalid. > That's fair, I moved these metrics to "Future work" with an explanation why. https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=69407758&selectedPageVersions=7&selectedPageVersions=6 Ismael