kfaraz commented on code in PR #16362:
URL: https://github.com/apache/druid/pull/16362#discussion_r1594909953
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java:
##########
@@ -880,7 +846,7 @@ public void testKillBatchSizeThree() throws Exception
Assert.assertEquals(Collections.emptyList(), observedUnusedSegments);
Assert.assertEquals(
- new KillTaskReport.Stats(4, 3, 4),
+ new KillTaskReport.Stats(4, 3, 0),
Review Comment:
Yes, that's true, markAsUsed / markAsUnused APIs don't seem to take any
locks right now. I think one way would be to wire up those APIs to use task
actions.
> if one uses kill job with REPLACE lock(default for concurrent locks),
shouldn't 2 REPLACE jobs be mutually exclusive? Maybe we shouldn't permit any
other lock besides REPLACE?
Yes, two REPLACE jobs are mutually exclusive. So point 4 is handled if we
use a REPLACE lock for a `kill` task.
> if we trying to append data to some segments, shouldn't they be 'used'? I
mean kill job should delete only unused segments, isn't it? Or I'm missing
something
That's true. I have just tried to list out all the possible corner case
scenarios (that I could think of) for posterity.
In point 5, I am trying to call out that it is actually okay to run a `kill`
task concurrently with `APPEND` jobs.
The only case when this might cause an issue is if while the append is in
progress, the kill task marks the segments being appended to as unused AND
deletes them. But since we are getting rid of the `markAsUnused` parameter from
`kill` tasks in this PR, this too is not a concern anymore.
(However, there is still the possibility of the markAsUnused API marking the
segments that we are appending to as unused and then `kill` task deleting them.
But this is already covered by point 2.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
