michaeljmarshall opened a new pull request #12846: URL: https://github.com/apache/pulsar/pull/12846
…ers map ### Motivation There is currently a bug that allows a producer to be removed from a topic's `producers` map, but not from the `ServerCnx#producers` map. As a consequence, two producers with the same name can connect at the same time. The fundamental issue here is with the definition of equality for the `Producer` class. This class's definition of equality has been an issue before. @merlimat fixed one such issue here: https://github.com/apache/pulsar/pull/11804. That fix addressed some of the problems, but I think we need to go a step further in making sure that the wrong producer cannot be removed from the topic's producers map. In the new test that I add with this PR, I show that it is still possible to remove the wrong producer. In the test, we essentially get two callbacks on the `getOrCreateTopic` future. Once that future succeeds, we create one producer and then the second fails and calls `producer.closeNow(true)`, which removes the successful producer from the topic. It does not remove the successful topic from the `ServerCnx` because of the changes introduced in https://github.com/apache/pulsar/pull/9256. The producer's `equals` method is only used when ensuring that the right producer is removed from the `producers` map and when determining if a producer is allowed to overwrite another producer. In my view, the simplest solution is to remove the overriding of `equals` and put the logic from `equals` into another method. This way removing a `producer` from a map is not too complicated (object equality is now reference equality), and we can still determine if a producer should override another. ### Modifications * Remove `Producer#equals` and `Producer#hashCode` to rely on the default object implementations. * Add `Producer#isSuccessorTo` to retain the logic required to determine if a producer is okay to overwrite an existing producer. ### Other Benefits When a producer is not present in a topic's producers map, the producer does not report metrics. ### Verifying this change I added a new test that shows a way to reproduce the issue. The new test fails on master but succeeds on my branch. There are also many tests that verify this code path to ensure this change does not introduce a regression. ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): no - The public API: no - The schema: no - The default values of configurations: no - The wire protocol: no - The rest endpoints: no - The admin cli options: no - Anything that affects deployment: no ### Documentation - [x] `no-need-doc` This is an internal bug fix. -- 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]
