poorbarcode commented on PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#issuecomment-1204292024

   Hi @asafm 
   > There is a way to solve this issue by making sure we're not defining 
metrics twice:
   We know that we plan to create only one Metrics instance per metic-prefix. 
So in that case, both TxLog and PendingAckStoreProvider will create one with 
its own prefix, and that's it. No need to verify it was created before. In the 
event a future developer will make a mistake, it will fail in the constructor 
in some test right since CollectorRegistry.register() will fail on a duplicate.
   
   That's a good way to do it, but this can make the code confuse:
   
   - When the Transaction Log Provider opens a Transaction Log, passed the 
`Histogram` to Transaction Log. That is OK.
   - When the Transaction Log close, remove the labels. That is ok too.
   
   But the Transaction Log Provider will hold all the `Collector` of Txn 
Buffered Writer, this is confusing


-- 
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]

Reply via email to