Dynamic PerQueryCPUMemAccountant Config on Servers

[vrajat]

PerQueryCPUMemAccountant can only be configured at startup of brokers and servers. This PR adds the ability to configure the object at runtime as well on servers. The accountant now implements PinotClusterConfigChangeListener and saves the changed configurations in its internal state.

As part of the change, all the internal state of PerQueryCPUMemAccountant has been changed to protected visibility from private visibility. This will enable customization by using derived classes and overriding methods.

Config params:

• pinot.query.scheduler.accounting.oom.enable.killing.query
• pinot.query.scheduler.accounting.query.killed.metric.enabled
• pinot.query.scheduler.accounting.oom.critical.heap.usage.ratio
• pinot.query.scheduler.accounting.oom.panic.heap.usage.ratio
• pinot.query.scheduler.accounting.publishing.jvm.heap.usage
• pinot.query.scheduler.accounting.sleep.ms=
• pinot.query.scheduler.accounting.oom.alarming.heap.usage.ratio
• pinot.query.scheduler.accounting.sleep.time.denominator
• pinot.query.scheduler.accounting.min.memory.footprint.to.kill.ratio
• pinot.query.scheduler.accounting.gc.backoff.count
• pinot.query.scheduler.accounting.oom.critical.heap.usage.ratio.delta.after.gc

Closes #16218

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 72.90640% with 55 lines in your changes missing coverage. Please review.

Project coverage is 63.19%. Comparing base (1a476de) to head (110bf79).
Report is 375 commits behind head on master.

Files with missing lines	Patch %	Lines
...ache/pinot/core/accounting/QueryMonitorConfig.java	77.23%	14 Missing and 14 partials ⚠️
...re/accounting/PerQueryCPUMemAccountantFactory.java	69.73%	14 Missing and 9 partials ⚠️
.../pinot/server/starter/helix/BaseServerStarter.java	0.00%	3 Missing ⚠️
.../spi/accounting/ThreadResourceUsageAccountant.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #16219      +/-   ##
============================================
+ Coverage     62.90%   63.19%   +0.28%     
+ Complexity     1386     1362      -24     
============================================
  Files          2867     2962      +95     
  Lines        163354   171158    +7804     
  Branches      24952    26210    +1258     
============================================
+ Hits         102755   108157    +5402     
- Misses        52847    54807    +1960     
- Partials       7752     8194     +442     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-11	63.16% <72.90%> (+0.29%)	⬆️
java-21	63.15% <72.90%> (+0.33%)	⬆️
skip-bytebuffers-false	?
skip-bytebuffers-true	?
temurin	63.19% <72.90%> (+0.28%)	⬆️
unittests	63.18% <72.90%> (+0.28%)	⬆️
unittests1	64.73% <74.00%> (+8.91%)	⬆️
unittests2	33.39% <0.00%> (-0.18%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vrajat]

@abhishekbafna @praveenc7

[somandal]

lgtm overall! left a minor comment

[somandal]

nit: I'd recommend adding an assert on what the existing value should be prior to the onChange call to ensure that the update actually went through. It'll be good to add this assert for all tests

e.g. for this:

    assertFalse(accountant.getWatcherTask().getQueryMonitorConfig().isOomKillQueryEnabled());
    accountant.getWatcherTask().onChange(
        Set.of(getFullyQualifiedConfigName(CommonConstants.Accounting.CONFIG_OF_OOM_PROTECTION_KILLING_QUERY)),
        CLUSTER_CONFIGS);
    assertTrue(accountant.getWatcherTask().getQueryMonitorConfig().isOomKillQueryEnabled());

[vrajat]

Done