Group commit IdealState updates

[xiangfu0]

We observed IdealState update becomes bottleneck for large table with high throughput segments update.

This PR shamelessly borrowed the idea/logic/code from: https://github.com/apache/helix/blob/master/helix-core/src/main/java/org/apache/helix/GroupCommit.java to group commit IdealState updates.

By running the IdealStateGroupCommitTest test, for my local setup, 2400 updates using 400 threads end up with 8 Zookeeper Writes, this is a significant improvement to relief zookeeper pressure.

[KKcorps]

Is it safe to modify the queue here while we are iterating on it?

[xiangfu0]

yes, this is processed in single thread

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 14.28571% with 174 lines in your changes missing coverage. Please review.

Project coverage is 57.90%. Comparing base (59551e4) to head (c63fe61).
Report is 1035 commits behind head on master.

Files with missing lines	Patch %	Lines
...inot/common/utils/helix/IdealStateGroupCommit.java	7.63%	121 Missing ⚠️
.../core/realtime/PinotLLCRealtimeSegmentManager.java	4.87%	39 Missing ⚠️
...ntroller/helix/core/PinotHelixResourceManager.java	62.50%	7 Missing and 2 partials ⚠️
...g/apache/pinot/common/utils/helix/HelixHelper.java	25.00%	3 Missing ⚠️
...g/apache/pinot/common/metrics/ControllerMeter.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #13976      +/-   ##
============================================
- Coverage     61.75%   57.90%   -3.85%     
- Complexity      207      219      +12     
============================================
  Files          2436     2613     +177     
  Lines        133233   143270   +10037     
  Branches      20636    21995    +1359     
============================================
+ Hits          82274    82963     +689     
- Misses        44911    53825    +8914     
- Partials       6048     6482     +434     

Flag	Coverage Δ
custom-integration1	<0.01% <0.00%> (-0.01%)	⬇️
integration	<0.01% <0.00%> (-0.01%)	⬇️
integration1	<0.01% <0.00%> (-0.01%)	⬇️
integration2	0.00% <0.00%> (ø)
java-11	57.86% <14.28%> (-3.85%)	⬇️
java-21	57.79% <14.28%> (-3.84%)	⬇️
skip-bytebuffers-false	57.89% <14.28%> (-3.85%)	⬇️
skip-bytebuffers-true	57.74% <14.28%> (+30.01%)	⬆️
temurin	57.90% <14.28%> (-3.85%)	⬇️
unittests	57.90% <14.28%> (-3.85%)	⬇️
unittests1	40.74% <8.02%> (-6.15%)	⬇️
unittests2	27.95% <8.86%> (+0.22%)	⬆️

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.

[Jackie-Jiang]

LGTM.
Can you double check the existing logic and ensure there is nothing rely on only the current update being processed? If there are, we can consider providing 2 fashions, one allow batching and one does not

[xiangfu0]

LGTM. Can you double check the existing logic and ensure there is nothing rely on only the current update being processed? If there are, we can consider providing 2 fashions, one allow batching and one does not

I think all the logics using commit method are moved this this sync best-effort kind of commit logic. I would say this is always better than the single commit.