Overall looks good to me. Only one thing I want to mention, should we use a new way to fix it, not only remove the original method? Because the issue indeed exists, and we need to fix it.
Thanks, Yong On Mon, 21 Aug 2023 at 09:37, Hang Chen <chenh...@apache.org> wrote: > +1 > > The interface introduces a heavy load on Zookeeper, and we need to remove > it. > For branch-4.15 and branch-4.16, we can keep the interface and remove > the implementation and remove the interface on the master branch. > > Thanks, > Hang > > Enrico Olivelli <eolive...@gmail.com> 于2023年8月20日周日 21:24写道： > > > > +1 > > > > Enrico > > > > Il Gio 17 Ago 2023, 10:06 horizonzy <horizo...@apache.org> ha scritto: > > > > > In https://github.com/apache/bookkeeper/pull/2805, it introduces > > > UnderReplicatedLedgersChangedCb to watch the zookeeper > `underreplication` > > > path. After a `underreplication` ledger mark is replicated, the > > > UnderReplicatedLedgersChangedCb will call back. > > > > > > In the callback, it will list all the `unnderreplication` ledgers to > record > > > the metrics `underReplicatedLedgersGuageValue`. It's a heavy operation > for > > > zookeeper. > > > > > > And in the pulsar, it introduces a deadlock after implementing the new > API > > > LedgerUnderreplicationManager#notifyUnderReplicationLedgerChanged > > > in PulsarLedgerUnderreplicationManager. see > > > https://github.com/apache/pulsar/pull/21010. > > > > > > I would suggest you remove underReplicatedLedgersChangedCb in Auditor. > If > > > the user wants to record the metrics, a scheduled task will be better > than > > > a zk watch event to trigger it. > > > >
