kfaraz commented on code in PR #15941:
URL: https://github.com/apache/druid/pull/15941#discussion_r1499376973
##########
server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java:
##########
@@ -96,362 +104,805 @@ public class KillUnusedSegmentsTest
private DataSegment nextDaySegment;
private DataSegment nextMonthSegment;
- private KillUnusedSegments target;
@Before
public void setup()
{
-
Mockito.doReturn(coordinatorDynamicConfig).when(params).getCoordinatorDynamicConfig();
- Mockito.doReturn(stats).when(params).getCoordinatorStats();
-
Mockito.doReturn(COORDINATOR_KILL_PERIOD).when(config).getCoordinatorKillPeriod();
-
Mockito.doReturn(DURATION_TO_RETAIN).when(config).getCoordinatorKillDurationToRetain();
-
Mockito.doReturn(INDEXING_PERIOD).when(config).getCoordinatorIndexingPeriod();
-
Mockito.doReturn(MAX_SEGMENTS_TO_KILL).when(config).getCoordinatorKillMaxSegments();
-
Mockito.doReturn(Duration.parse("PT3154000000S")).when(config).getCoordinatorKillBufferPeriod();
+ segmentsMetadataManager = new TestSegmentsMetadataManager();
+ overlordClient = new TestOverlordClient();
- Mockito.doReturn(Collections.singleton(DATASOURCE))
-
.when(coordinatorDynamicConfig).getSpecificDataSourcesToKillUnusedSegmentsIn();
+ // These two can definitely be part of setup()
+ configBuilder = new TestDruidCoordinatorConfig.Builder()
+ .withCoordinatorIndexingPeriod(INDEXING_PERIOD)
+ .withCoordinatorKillPeriod(COORDINATOR_KILL_PERIOD)
+ .withCoordinatorKillDurationToRetain(DURATION_TO_RETAIN)
+ .withCoordinatorKillMaxSegments(MAX_SEGMENTS_TO_KILL)
+ .withCoordinatorKillBufferPeriod(BUFFER_PERIOD);
+ paramsBuilder =
DruidCoordinatorRuntimeParams.newBuilder(DateTimes.nowUtc());
+ }
- final DateTime now = DateTimes.nowUtc();
+ /**
+ * The buffer periood and duration to retain influence kill behavior.
+ */
+ @Test
+ public void testDefaults()
+ {
+ final DateTime sixtyDaysAgo = NOW.minusDays(60);
- yearOldSegment = createSegmentWithEnd(now.minusDays(365));
Review Comment:
Just calling this out in case it got missed in the refactor.
The original segments had their _end_ a year ago from now, a month ago from
now and so on.
The new intervals `YEAR_OLD`, `MONTH_OLD` actually have their end at now.
Please ensure that the tests have been updated to incorporate that.
##########
server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java:
##########
@@ -44,50 +53,49 @@
import org.joda.time.Period;
import org.junit.Assert;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Answers;
-import org.mockito.ArgumentMatchers;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.junit.MockitoJUnitRunner;
+import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.stream.Collectors;
+import java.util.Set;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyInt;
-import static org.mockito.ArgumentMatchers.anyString;
/**
- *
+ * Unit tests to test the {@link KillUnusedSegments} duty.
*/
-@RunWith(MockitoJUnitRunner.class)
public class KillUnusedSegmentsTest
{
- private static final Duration INDEXING_PERIOD = Duration.standardSeconds(1);
- private static final Duration COORDINATOR_KILL_PERIOD =
Duration.standardSeconds(1);
- private static final Duration DURATION_TO_RETAIN = Duration.standardDays(1);
+ private static final Logger log = new Logger(KillUnusedSegmentsTest.class);
+ private static final Duration INDEXING_PERIOD = Duration.standardSeconds(0);
+ private static final Duration COORDINATOR_KILL_PERIOD =
Duration.standardSeconds(0);
+ private static final Duration DURATION_TO_RETAIN =
Duration.standardHours(36);
+ private static final Duration BUFFER_PERIOD = Duration.standardSeconds(1);
private static final int MAX_SEGMENTS_TO_KILL = 10;
- private static final String DATASOURCE = "DS1";
-
- @Mock
- private SegmentsMetadataManager segmentsMetadataManager;
- @Mock
- private OverlordClient overlordClient;
- @Mock(answer = Answers.RETURNS_DEEP_STUBS)
- private DruidCoordinatorConfig config;
-
- @Mock
- private CoordinatorRunStats stats;
- @Mock
- private DruidCoordinatorRuntimeParams params;
- @Mock
- private CoordinatorDynamicConfig coordinatorDynamicConfig;
+ private static final String DS1 = "DS1";
+ private static final String DS2 = "DS2";
+
+ private TestSegmentsMetadataManager segmentsMetadataManager;
+
+ private TestOverlordClient overlordClient;
+
+ private TestDruidCoordinatorConfig.Builder configBuilder;
+
+ private DruidCoordinatorRuntimeParams.Builder paramsBuilder;
+
+ private final CoordinatorDynamicConfig.Builder dynamicConfigBuilder =
CoordinatorDynamicConfig.builder();
+
+ private static final DateTime NOW = DateTimes.nowUtc();
+ private static final Interval YEAR_OLD = new Interval(Period.days(1),
NOW.minusDays(365));
Review Comment:
More apt to call these `YEAR_TILL_DATE`, `MONTH_TILL_DATE` or something.
`YEAR_OLD` is ambiguous.
--
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]
