kfaraz commented on code in PR #15994:
URL: https://github.com/apache/druid/pull/15994#discussion_r1522442926
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java:
##########
@@ -569,10 +735,10 @@ public void
testKillMultipleUnusedSegmentsWithDifferentMaxUsedStatusLastUpdatedT
final List<DataSegment> unusedSegments =
getMetadataStorageCoordinator().retrieveUnusedSegmentsForInterval(
- DATA_SOURCE,
- umbrellaInterval,
- null,
- null
+ DATA_SOURCE,
+ umbrellaInterval,
+ null,
+ null
);
Review Comment:
Indentation seems off. Closing brace should have smaller indentation than
the preceding args.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java:
##########
@@ -173,62 +227,168 @@ public void testKillUnusedSegmentsWithUsedLoadSpec()
throws Exception
null
);
- Assert.assertEquals(ImmutableSet.of(), new
HashSet<>(observedUnusedSegments));
+ Assert.assertEquals(ImmutableSet.of(segment5), new
HashSet<>(observedUnusedSegments));
}
-
@Test
- public void testKillWithMarkUnused() throws Exception
+ public void testKillSegmentsWithVersionsAndLimit() throws Exception
{
- final String version = DateTimes.nowUtc().toString();
- final Set<DataSegment> segments = ImmutableSet.of(
- newSegment(Intervals.of("2019-01-01/2019-02-01"), version),
- newSegment(Intervals.of("2019-02-01/2019-03-01"), version),
- newSegment(Intervals.of("2019-03-01/2019-04-01"), version),
- newSegment(Intervals.of("2019-04-01/2019-05-01"), version)
+ final DateTime now = DateTimes.nowUtc();
+ final String v1 = now.toString();
+ final String v2 = now.minusHours(2).toString();
+ final String v3 = now.minusHours(3).toString();
+
+ final DataSegment segment1 =
newSegment(Intervals.of("2019-01-01/2019-02-01"), v1);
+ final DataSegment segment2 =
newSegment(Intervals.of("2019-02-01/2019-03-01"), v1);
+ final DataSegment segment3 =
newSegment(Intervals.of("2019-03-01/2019-04-01"), v1);
+ final DataSegment segment4 =
newSegment(Intervals.of("2019-01-01/2019-02-01"), v2);
+ final DataSegment segment5 =
newSegment(Intervals.of("2019-01-01/2019-02-01"), v3);
+
+ final Set<DataSegment> segments = ImmutableSet.of(segment1, segment2,
segment3, segment4, segment5);
+
+ Assert.assertEquals(segments,
getMetadataStorageCoordinator().commitSegments(segments));
+ Assert.assertEquals(
+ segments.size(),
+ getSegmentsMetadataManager().markSegmentsAsUnused(
+
segments.stream().map(DataSegment::getId).collect(Collectors.toSet())
+ )
);
- final Set<DataSegment> announced =
getMetadataStorageCoordinator().commitSegments(segments);
- Assert.assertEquals(segments, announced);
+ final KillUnusedSegmentsTask task = new KillUnusedSegmentsTask(
+ null,
+ DATA_SOURCE,
+ Intervals.of("2018/2020"),
+ ImmutableList.of(v1),
+ null,
+ false,
+ 3,
+ 2,
+ null
+ );
- Assert.assertTrue(
- getSegmentsMetadataManager().markSegmentAsUnused(
- newSegment(Intervals.of("2019-02-01/2019-03-01"), version).getId()
+ Assert.assertEquals(TaskState.SUCCESS,
taskRunner.run(task).get().getStatusCode());
+ Assert.assertEquals(
+ new KillTaskReport.Stats(2, 1, 0),
+ getReportedStats()
+ );
+
+ final List<DataSegment> observedUnusedSegments =
getMetadataStorageCoordinator().retrieveUnusedSegmentsForInterval(
+ DATA_SOURCE,
+ Intervals.of("2018/2020"),
+ null,
+ null
+ );
+
+ Assert.assertEquals(ImmutableSet.of(segment3, segment4, segment5), new
HashSet<>(observedUnusedSegments));
+ }
+
+ @Test
+ public void testKillWithNonExistentVersion() throws Exception
+ {
+ final DateTime now = DateTimes.nowUtc();
+ final String v1 = now.toString();
+ final String v2 = now.minusHours(2).toString();
+ final String v3 = now.minusHours(3).toString();
+
+ final DataSegment segment1 =
newSegment(Intervals.of("2019-01-01/2019-02-01"), v1);
+ final DataSegment segment2 =
newSegment(Intervals.of("2019-02-01/2019-03-01"), v1);
+ final DataSegment segment3 =
newSegment(Intervals.of("2019-03-01/2019-04-01"), v1);
+ final DataSegment segment4 =
newSegment(Intervals.of("2019-01-01/2019-02-01"), v2);
+ final DataSegment segment5 =
newSegment(Intervals.of("2019-01-01/2019-02-01"), v3);
+
+ final Set<DataSegment> segments = ImmutableSet.of(segment1, segment2,
segment3, segment4, segment5);
+
+ Assert.assertEquals(segments,
getMetadataStorageCoordinator().commitSegments(segments));
+ Assert.assertEquals(
+ segments.size(),
+ getSegmentsMetadataManager().markSegmentsAsUnused(
+
segments.stream().map(DataSegment::getId).collect(Collectors.toSet())
)
);
- final KillUnusedSegmentsTask task =
- new KillUnusedSegmentsTask(
- null,
- DATA_SOURCE,
- Intervals.of("2019-03-01/2019-04-01"),
- null,
- true,
- null,
- null,
- null
- );
+ final KillUnusedSegmentsTask task = new KillUnusedSegmentsTask(
+ null,
+ DATA_SOURCE,
+ Intervals.of("2018/2020"),
+ ImmutableList.of(now.plusDays(100).toString()),
+ null,
+ false,
+ 3,
+ 2,
+ null
+ );
Assert.assertEquals(TaskState.SUCCESS,
taskRunner.run(task).get().getStatusCode());
+ Assert.assertEquals(
+ new KillTaskReport.Stats(0, 1, 0),
+ getReportedStats()
+ );
- final List<DataSegment> unusedSegments =
- getMetadataStorageCoordinator().retrieveUnusedSegmentsForInterval(
- DATA_SOURCE,
- Intervals.of("2019/2020"),
- null,
- null
- );
+ final List<DataSegment> observedUnusedSegments =
getMetadataStorageCoordinator().retrieveUnusedSegmentsForInterval(
+ DATA_SOURCE,
+ Intervals.of("2018/2020"),
+ null,
+ null
+ );
-
Assert.assertEquals(ImmutableList.of(newSegment(Intervals.of("2019-02-01/2019-03-01"),
version)), unusedSegments);
- Assertions.assertThat(
- getMetadataStorageCoordinator()
- .retrieveUsedSegmentsForInterval(DATA_SOURCE,
Intervals.of("2019/2020"), Segments.ONLY_VISIBLE)
- ).containsExactlyInAnyOrder(
- newSegment(Intervals.of("2019-01-01/2019-02-01"), version),
- newSegment(Intervals.of("2019-04-01/2019-05-01"), version)
+ Assert.assertEquals(segments, new HashSet<>(observedUnusedSegments));
+ }
+
+ /**
+ * {@code segment1}, {@code segment2} and {@code segment3} have different
versions, but share the same load spec.
+ * {@code segment1} and {@code segment2} are unused segments, while {@code
segment3} is a used segment.
+ * When a kill task is submitted, the unused segments {@code segment1} and
{@code segment2} should be deleted from the
+ * metadata store, but should be retained in deep storage as the load spec
is used by {@code segment3}.
+ */
+ @Test
+ public void testKillUnusedSegmentsWithUsedLoadSpec() throws Exception
+ {
+ final DateTime now = DateTimes.nowUtc();
+ final String v1 = now.toString();
+ final String v2 = now.minusHours(2).toString();
+ final String v3 = now.minusHours(3).toString();
+
+ final DataSegment segment1 =
newSegment(Intervals.of("2019-01-01/2019-02-01"), v1, ImmutableMap.of("foo",
"1"));
+ final DataSegment segment2 =
newSegment(Intervals.of("2019-02-01/2019-03-01"), v2, ImmutableMap.of("foo",
"1"));
+ final DataSegment segment3 =
newSegment(Intervals.of("2019-03-01/2019-04-01"), v3, ImmutableMap.of("foo",
"1"));
Review Comment:
Many of the tests seem to be using similar segments, versions, etc. Do you
think some of this can go into the `setup` method?
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java:
##########
@@ -101,10 +102,11 @@ public void testKill() throws Exception
Assert.assertEquals(TaskState.SUCCESS,
taskRunner.run(task).get().getStatusCode());
final List<DataSegment> unusedSegments =
getMetadataStorageCoordinator().retrieveUnusedSegmentsForInterval(
- DATA_SOURCE,
- Intervals.of("2019/2020"),
- null,
- null
+ DATA_SOURCE,
+ Intervals.of("2019/2020"),
+ null,
+ null,
+ null
);
Review Comment:
Indentation seems off when looking at the args in the previous lines.
--
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]
