michaeljmarshall opened a new pull request, #4300:
URL: https://github.com/apache/bookkeeper/pull/4300

   Descriptions of the changes in this PR:
   
   There is an issue in the `ThreadRegistry` that causes incomplete Prometheus 
metrics reporting. It is trivial to reproduce:
   
   1. Start bookies with multiple journal directories.
   2. Start multiple clients writing to the bookies.
   3. Observe that the `bookkeeper_server_ADD_ENTRY_count` does not increment 
at a similar rate to the journal's add entry rate.
   
   The bug can be understood abstractly by considering the following:
   
   1. The `threadPoolMap` in `ThreadRegistry` goes from thread id to 
`ThreadPoolThread`.
   2. The `ThreadPoolThread` is defined by the thread pool and the configured 
id.
   3. Every time we set the id for step 2, we use `0`.
   4. When we have multiple journals, we call the same 
`ThreadRegistry.register(journalThreadName, 0);` method multiple times.
   5. The `threadPoolMap` ends up with multiple identical values for different 
keys.
   6. `ThreadScopedDataSketchesStatsLogger#getStatsLogger` stores the identical 
values in the `provider.opStats` map and ultimately overwrites map values, 
which leads to an under-reporting of metrics.
   
   ### Motivation
   
   Correct bookkeeper metrics reporting by ensuring that threads are properly 
registered in the `ThreadRegistry`.
   
   ### Changes
   
   I updated several usages of the `ThreadRegistry#register` method that seemed 
suspect. I'll need confirmation from others that these are correct changes.
   
   I need help identifying the best place to test with multiple journal 
entries. The existing prometheus tests are not sophisticated enough to cover 
this kind of bug. As such, I added assertions to the code to verify the 
behavior when `-ea` is passed to the JVM. In my local testing, these changes 
were sufficient.


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