TakaHiR07 commented on PR #23977: URL: https://github.com/apache/pulsar/pull/23977#issuecomment-2664943665
> > > Great catch @TakaHiR07. What is the current impact of this in Pulsar & Bookkeeper (which is using PulsarLedgerManager in the Pulsar distribution of Bookkeeper)? > > > > > > One impact is all the asyncOpenLedgerNoRecovery in pulsar can not register successful MetadataListener. The code is here: https://github.com/apache/bookkeeper/blob/606db747eae9856fed0aeb3f16ef01e7c9254e26/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java#L95-L105 > > I am not sure whether other place use PulsarLedgerManager and register zk listener. > > I wonder what parts of the metadata could change. My guess is LAC (lastAddConfirmed) and length based on this: https://github.com/apache/bookkeeper/blob/54bdc0d60b32830b513089167cee67f52f4735eb/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java#L367-L370 . I would assume that this would be relevant when the ledger is in recovery state. States: https://github.com/apache/bookkeeper/blob/2192caaf9738cf4efb799647cc5a5f68bf1823b2/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/LedgerMetadata.java#L154-L167 @lhotari I think if ledger is in recover state, LAC would be changed. But we should not use asyncOpenLedgerNoRecovery to register zk metadata listener, instead should use asyncOpenLedger to update ledgerHandler's metadata. This is no problem. But if ledger is already closed, and then trigger bookkeeper auto-recovery because of disk error, ledger's quorum would be changed. Actually, the issue is found when I fix another issue, you can see here. https://github.com/apache/pulsar/pull/21552 -- 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]
