lhotari commented on pull request #7937: URL: https://github.com/apache/pulsar/pull/7937#issuecomment-686925505
@Technoboy- @codelipenghui @jiazhai Hi, I was looking at the fix for the NPE. It looks like the fix would silently by-pass any operations when dispatcher is null. Wouldn't skipping the operations cause issues in consistency? Is it ok to skip the operations? I guess it is, but I'm just trying to understand Pulsar coding style by following how it's developed. Therefore I'm asking these questions. Perhaps some day I'd be able to contribute too. :) Just thinking out loud here. I don't mean to be rude. There are 16 times that `dispatcher != null` is in the code of PersistentSubscription. Why is dispatcher null in the first place? Would there be a need to improve the design, for example have some kind of state machine / model that would help organize the code in a better way? It seems that the design isn't optimal if each time you call a method on dispatcher, you would have to do a null check. I'd assume that some kind of proxy pattern would encapsulate things in a better way. (Another unrelated observation is that there are a lot of synchronized methods in the PersistentSubscription class, however I guess there isn't a lockfree design in Pulsar in general.) ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
