[Bugfix] delete segment API always deletes consuming segments

[tibrewalpratik17]

label: bugfix

At present, while using DELETE API "/segments/{tableName}/choose" we always end up deleting consuming segments. This is because of using common segments selection logic across GET and DELETE APIs.

The root cause is the following line in the selection logic. https://github.com/apache/pinot/blob/f251f00182a75dd79f2a7388546e92fb2a1658e6/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java#L951-L954

For consuming segment, we don't have startTime and endTime in ZK populated. For fixing, we have introduced a variable excludeConsuming in the selection logic to still use the same method for GET and DELETE APIs. For DELETE, we mark excludeConsuming as true.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.74%. Comparing base (59551e4) to head (a4a21b6). Report is 1239 commits behind head on master.

Files with missing lines	Patch %	Lines
...ntroller/helix/core/PinotHelixResourceManager.java	0.00%	1 Missing and 2 partials :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #14293      +/-   ##
============================================
+ Coverage     61.75%   63.74%   +1.99%     
- Complexity      207     1556    +1349     
============================================
  Files          2436     2659     +223     
  Lines        133233   145451   +12218     
  Branches      20636    22221    +1585     
============================================
+ Hits          82274    92714   +10440     
- Misses        44911    45881     +970     
- Partials       6048     6856     +808     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (+99.99%)	:arrow_up:
integration	100.00% <ø> (+99.99%)	:arrow_up:
integration1	100.00% <ø> (+99.99%)	:arrow_up:
integration2	0.00% <ø> (ø)
java-11	63.72% <0.00%> (+2.01%)	:arrow_up:
java-21	63.62% <0.00%> (+2.00%)	:arrow_up:
skip-bytebuffers-false	63.73% <0.00%> (+1.98%)	:arrow_up:
skip-bytebuffers-true	63.60% <0.00%> (+35.87%)	:arrow_up:
temurin	63.74% <0.00%> (+1.99%)	:arrow_up:
unittests	63.73% <0.00%> (+1.99%)	:arrow_up:
unittests1	55.38% <ø> (+8.49%)	:arrow_up:
unittests2	34.25% <0.00%> (+6.51%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[tibrewalpratik17]

I'd suggest modifying the logic to not selecting consuming segment when end timestamp is specified. Essentially counting the consuming segment as always match the start timestamp but not match the end timestamp. We do want to keep the behavior consistent in GET and DELETE

Sounds good! Updated accordingly.