[Issue 7878][broker]Fix NPE when acknowledge messages at the broker side

[Technoboy-]

No description provided.

[lhotari]

@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.)

[codelipenghui]

@lhotari Thanks for the comment. This will not cause issues in consistency, this PR only add check for the acknowledge method. Currently, the acknowledgment is in a try best way.

for example have some kind of state machine / model that would help organize the code in a better way?

Sounds good. If you are interested in this part, feel free to push forward.

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.

Yes, maybe this is the part that can be improved. Some seem to be unnecessary.