Relax Segment Operation Throttler Permit limitation to zero

[J-HowHuang]

Description

Currently we disallow users to set throttler permit to zero (e.g. pinot.server.max.segment.preprocess.parallelism=0). Relaxing this limitation so that these throttlers can be used as kill switches functionally, to disable certain segment operations.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.46%. Comparing base (5b0b38c) to head (6cd70d1).
⚠️ Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
...ent/local/utils/SegmentOperationsThrottlerSet.java	20.00%	2 Missing and 2 partials ⚠️
...egment/local/utils/SegmentOperationsThrottler.java	50.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18366      +/-   ##
============================================
+ Coverage     63.43%   63.46%   +0.03%     
  Complexity     1683     1683              
============================================
  Files          3253     3253              
  Lines        198841   198884      +43     
  Branches      30795    30798       +3     
============================================
+ Hits         126136   126223      +87     
+ Misses        62625    62589      -36     
+ Partials      10080    10072       -8     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-21	63.46% <33.33%> (+0.03%)	⬆️
temurin	63.46% <33.33%> (+0.03%)	⬆️
unittests	63.46% <33.33%> (+0.03%)	⬆️
unittests1	55.38% <0.00%> (+0.02%)	⬆️
unittests2	35.01% <33.33%> (+0.01%)	⬆️

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.

[yashmayya]

I don't like this test, it has a lot of design smells IMO and could be flaky. We shouldn't be polling on Thread.State.WAITING, it's an internal thread state and not a synchronization primitive. Also this comment:

// Wait until the acquirer is parked inside acquire(). availablePermits() goes to -1 once it
// has decremented but not yet been granted a permit (AdjustableSemaphore behavior), so poll
// for that condition.

doesn't seem to match the code? Can't we just check the throttler queue length or something instead? This test feels like overkill for what's being asserted.

[J-HowHuang]

I think the blocking part is trivial, might as well remove the test that tests blocking. I'll keep the test to verify the unblocking part.

[xiangfu0]

Opened a docs follow-up PR for this change: pinot-contrib/pinot-docs#783