BewareMyPower commented on PR #20800: URL: https://github.com/apache/pulsar/pull/20800#issuecomment-1635413884
Sorry I misunderstand the changes in your PR that they are not unrelated so I edited the PR description again. https://github.com/apache/pulsar/pull/20597 not only fixes the NPE, **but also fixes the bug that `PersistentSubscription` will be queried from the `PersistentTopic` even if the subscription is a replicator subscription.** While the original PR description only mentioned: > save PersistentTopic in PersistentMessageExpiryMonitor field. > > ensure when call expireMessages(Position messagePosition) only check topic LastPosition by topic. not null subscription. It didn't mention why it needs to modify the `PersistentTopicsBase.java` in addition to the changes of the `PersistentMessageExpiryMonitor#topic` field and the `PersistentMessageExpiryMonitor#expireMessages` method. P.S. I didn't blame the author (you) for that. I just pointed out the issue and why I misunderstood it. The reviewer should also be responsible to point out the missed description. > but not with some insult words @lifepuzzlefun I apologize if you thought my words insulted you. I only aimed at the code when I said "The code above is such a mess.", which I removed from the PR description now. > 这里的代码风格是 从first init comment 提交的时候带过来的。 Yes you used a similar code style with the code in the first commit, but I think the original code is such a mess as well. Though I realized the words I used are not proper now. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
