Remove segments with empty download url in UpsertCompactionTask

[tibrewalpratik17]

bugfix

During UpsertCompactionTask generation, if segments are missing downloadUrl in ZK, even those are processed and pushed to task-execution phase. While downloading the file from deep-store, it throws an NPE and errors out.

Removing these segments in the task-generation phase itself so that other relevant segments can be processed until maxTasks is reached.

[codecov-commenter]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (6cc1915) 61.65% compared to head (bde29a6) 34.77%.
Report is 2 commits behind head on master.

Files	Patch %	Lines
...psertcompaction/UpsertCompactionTaskGenerator.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #12320       +/-   ##
=============================================
- Coverage     61.65%   34.77%   -26.88%     
+ Complexity     1152      951      -201     
=============================================
  Files          2421     2345       -76     
  Lines        131872   128136     -3736     
  Branches      20346    19789      -557     
=============================================
- Hits          81308    44563    -36745     
- Misses        44600    80384    +35784     
+ Partials       5964     3189     -2775     

Flag	Coverage Δ
custom-integration1	<0.01% <0.00%> (ø)
integration	<0.01% <0.00%> (ø)
integration1	<0.01% <0.00%> (ø)
integration2	0.00% <0.00%> (ø)
java-11	<0.01% <0.00%> (-61.60%)	⬇️
java-21	34.77% <0.00%> (-26.76%)	⬇️
skip-bytebuffers-false	34.76% <0.00%> (-26.88%)	⬇️
skip-bytebuffers-true	34.75% <0.00%> (-26.74%)	⬇️
temurin	34.77% <0.00%> (-26.88%)	⬇️
unittests	46.59% <ø> (-15.06%)	⬇️
unittests1	46.59% <ø> (-0.19%)	⬇️
unittests2	?

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.

[walterddr]

why would a download url be empty for a minion task?

[tibrewalpratik17]

why would a download url be empty for a minion task?

There are scenarios when downloadURL can be empty (when deepstore upload timed out during segment commit or something). There are ways to enable retries on them but during that period the ZK config has downloadURL has empty.

Now if the minion tasks runs during this period, the downloadURL is fetched as null and passed on to task-execution phase where it throws NPE. This patch doesn't allow these segments to go to the task-execution phase itself and stops them in task-generation phase.

[ankitsultana]

lgtm. Ideally we could also emit a metric but I am okay without it as well.

[tibrewalpratik17]

@walterddr can you help approve / merge this PR?