lhotari commented on PR #23901: URL: https://github.com/apache/pulsar/pull/23901#issuecomment-2647223699
> I'm currently reviewing this PR so I can have a better understanding on it. Generally, we need a careful review on such a huge PR. But it's still hard. From my experiences, it's very easy to hide some bugs not noticed in a huge PR. @BewareMyPower I don't disagree with you about the size of a PR. Large PRs are harder to review and there are more chances that the changes cause regressions. It's also good to notice that if bugs pass CI and all regression test suites, it's also a bug in the test suites, which should also be addressed. Some problems require larger PRs than others. If we start bashing contributors that fix broader issues, it's going to kill this project. I hope that instead of complaining about large PRs and regressions, please put the effort in improving test suites to catch the regressions and put the effort in reviews when it's needed. That's simply more productive for everyone. > > we also found a memory leak due to ByteBuf objects not released. > > It's in SN's private repo, let me ping you in that issue. BTW, @shibd I think we should report it in the Apache repo as well. Memory leaks could also be seen as a failure of our test suites to catch them. For addressing memory leaks, it would be possible to run unit tests with paranoid level and fail the tests if any leaks are detected. This would be an effective way to catch such problems as early as possible. -- 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]
