On Mon, 28 Jul 2025 at 22:06, PengHui Li <[email protected]> wrote: > > Why was there such urgency in merging a proposal and implementation that > introduces significant changes to the hot path?
The PR https://github.com/apache/pulsar/pull/24363 has been in review since May 29th. It was simply waiting for PIP-430 to pass. There have been plenty of opportunities for reviews before these changes have been merged. My motivation for merging PR 24363 is that I'd like to rebase the changes I have for the next part of the PIP-430 implementation. Please notice that these changes are going to the master branch and won't impact released versions of Pulsar. > Both the proposal and implementation PR received just one approval, despite > containing a substantial number of changes: +2,315 / −1,450 lines. Since > that initial approval, 11 additional commits have been pushed to the PR. > > Who reviewed these 11 commits? > Were they adequately reviewed given the scope and impact of the changes? These are the 11 commits that happened after Matteo's review: https://github.com/lhotari/pulsar/compare/365fc07f727044a614fc35d43f6b5f6b6a157108...b62ccdddbf9b787b0b1e9e584f26e57f00469f33 These aren't substancial changes. They are addressing Matteo's review comments and fixing test failures. I think that PR has been adequately reviewed by Matteo. If you have concerns about this work, please let me know. When something gets merged, it doesn't mean that we have set things in stone at that point. > > Thank you, Penghui. This is a vote thread, so we can handle this concern > separately once the work proceeds in implementation PRs. > > Ok, my bad. I will give my explicit -1 next time. The proposal needs to add > the configuration change, no? Sure, it will be helpful to raise feedback during the discussion phase of a PIP. In PIP-430, there's already configuration options for selecting the way how caching works. I don't currently think that other configuration options than what PIP-430 has would be needed in practice. > Please take > https://github.com/apache/pulsar/pull/15955/files#diff-1e63fb74fd8bcabf5320f10d72ea893e1ce72cbb5b7f55a49460674d905c09c7R1063 > for example. Yes, that solution has been considered while designing PIP-430. -Lari
