[GitHub] [kafka] jsancio commented on pull request #11109: KAFKA-13113: Support unregistering Raft listeners

[GitBox <gi...@apache.org>]

jsancio commented on pull request #11109:
URL: https://github.com/apache/kafka/pull/11109#issuecomment-885800314


   > I don't see why we need this context object. The listener itself is the context object. It should be able to unregister itself by just calling `RaftClient#unregister(this)`
   > 
   > You need a context object in non-object-oriented languages like C, where you can't call `Foo#bar(a, b)`, but you have to call `bar(Foo foo, a, b)` Since Java is object-oriented, the context is in `Foo` already, so no need for a helper object.
   
   Yeah. At a high-level we need to support two requirements:
   1. Unregistering a Listener.
   2. The Controller needs to know if a given "handleCommit", "handleSnapshot" or "handleLeaderChange" is for the previous registration or the current registration.
   
   With the implementation in this PR the controller can do the following to determine if those events are for the current registration or the previous registration:
   https://github.com/apache/kafka/pull/11116/files#diff-77dc2adb187fd078084644613cff2b53021c8a5fbcdcfa116515734609d1332a
   
   If we don't expose the `ListenerContext` then the Controller needs to use a different `Listener` instance in the new registration so that it can determine if the "handleCommit", "handleSnapshot" or "handleLeaderChange" are for the current registration. With this other solution it is up to the user (Controller) to guarantee this and the RaftClient cannot help with this check.
   
   I am okay with either solution as there are pros and cons to both.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org