github-advanced-security[bot] commented on code in PR #18860:
URL: https://github.com/apache/druid/pull/18860#discussion_r2661179004
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamSupervisorSpecTest.java:
##########
@@ -1593,6 +1627,175 @@
originalSpec.validateSpecUpdateTo(proposedSpecSameSource);
}
+ @Test
+ public void
test_dynamicAllocationNotice_skipsScalingAndEmitsReason_ifTasksArePublishing()
throws InterruptedException
+ {
+ EasyMock.expect(spec.getId()).andReturn(SUPERVISOR).anyTimes();
+
EasyMock.expect(spec.getSupervisorStateManagerConfig()).andReturn(supervisorConfig).anyTimes();
+
EasyMock.expect(spec.getDataSchema()).andReturn(getDataSchema()).anyTimes();
+ EasyMock.expect(spec.getIoConfig()).andReturn(getIOConfig(1,
true)).anyTimes();
+
EasyMock.expect(spec.getTuningConfig()).andReturn(getTuningConfig()).anyTimes();
+ EasyMock.expect(spec.getEmitter()).andReturn(emitter).anyTimes();
+ EasyMock.expect(spec.isSuspended()).andReturn(false).anyTimes();
+ EasyMock.replay(spec);
+
+
EasyMock.expect(ingestionSchema.getIOConfig()).andReturn(seekableStreamSupervisorIOConfig).anyTimes();
+
EasyMock.expect(ingestionSchema.getDataSchema()).andReturn(dataSchema).anyTimes();
+
EasyMock.expect(ingestionSchema.getTuningConfig()).andReturn(seekableStreamSupervisorTuningConfig).anyTimes();
+ EasyMock.replay(ingestionSchema);
+
+
EasyMock.expect(taskMaster.getTaskRunner()).andReturn(Optional.absent()).anyTimes();
+
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.absent()).anyTimes();
+ EasyMock.replay(taskMaster);
+
+ StubServiceEmitter dynamicActionEmitter = new StubServiceEmitter();
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(10);
+
+ LagBasedAutoScaler autoScaler = new LagBasedAutoScaler(
+ supervisor,
+ DATASOURCE,
+ mapper.convertValue(
+ getScaleOutProperties(2),
+ LagBasedAutoScalerConfig.class
+ ),
+ spec,
+ dynamicActionEmitter
+ );
+
+ supervisor.addTaskGroupToPendingCompletionTaskGroup(
+ 0,
+ ImmutableMap.of("0", "0"),
+ null,
+ null,
+ Set.of("dummyTask"),
+ Collections.emptySet()
+ );
+
+ supervisor.start();
+ autoScaler.start();
+
+ supervisor.runInternal();
+ Thread.sleep(1000); // ensure a dynamic allocation notice completes
+
+ Assert.assertEquals(1, supervisor.getIoConfig().getTaskCount().intValue());
+ Assert.assertTrue(
+ dynamicActionEmitter
+
.getMetricEvents(SeekableStreamSupervisor.AUTOSCALER_REQUIRED_TASKS_METRIC)
+ .stream()
+ .map(metric ->
metric.getUserDims().get(SeekableStreamSupervisor.AUTOSCALER_SKIP_REASON_DIMENSION))
+ .filter(Objects::nonNull)
+ .anyMatch("There are tasks pending completion"::equals)
+ );
+
+
emitter.verifyNotEmitted(SeekableStreamSupervisor.AUTOSCALER_SCALING_TIME_METRIC);
+ autoScaler.reset();
+ autoScaler.stop();
+ }
+
+ @Test
+ public void test_dynamicAllocationNotice_skips_whenSupervisorSuspended()
throws InterruptedException
+ {
+ EasyMock.expect(spec.getId()).andReturn(SUPERVISOR).anyTimes();
+
EasyMock.expect(spec.getSupervisorStateManagerConfig()).andReturn(supervisorConfig).anyTimes();
+
+
EasyMock.expect(spec.getDataSchema()).andReturn(getDataSchema()).anyTimes();
Review Comment:
## Deprecated method or constructor invocation
Invoking [SeekableStreamSupervisorSpec.getDataSchema](1) should be avoided
because it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10624)
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamSupervisorSpecTest.java:
##########
@@ -1593,6 +1627,175 @@
originalSpec.validateSpecUpdateTo(proposedSpecSameSource);
}
+ @Test
+ public void
test_dynamicAllocationNotice_skipsScalingAndEmitsReason_ifTasksArePublishing()
throws InterruptedException
+ {
+ EasyMock.expect(spec.getId()).andReturn(SUPERVISOR).anyTimes();
+
EasyMock.expect(spec.getSupervisorStateManagerConfig()).andReturn(supervisorConfig).anyTimes();
+
EasyMock.expect(spec.getDataSchema()).andReturn(getDataSchema()).anyTimes();
+ EasyMock.expect(spec.getIoConfig()).andReturn(getIOConfig(1,
true)).anyTimes();
+
EasyMock.expect(spec.getTuningConfig()).andReturn(getTuningConfig()).anyTimes();
+ EasyMock.expect(spec.getEmitter()).andReturn(emitter).anyTimes();
+ EasyMock.expect(spec.isSuspended()).andReturn(false).anyTimes();
+ EasyMock.replay(spec);
+
+
EasyMock.expect(ingestionSchema.getIOConfig()).andReturn(seekableStreamSupervisorIOConfig).anyTimes();
+
EasyMock.expect(ingestionSchema.getDataSchema()).andReturn(dataSchema).anyTimes();
+
EasyMock.expect(ingestionSchema.getTuningConfig()).andReturn(seekableStreamSupervisorTuningConfig).anyTimes();
+ EasyMock.replay(ingestionSchema);
+
+
EasyMock.expect(taskMaster.getTaskRunner()).andReturn(Optional.absent()).anyTimes();
+
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.absent()).anyTimes();
+ EasyMock.replay(taskMaster);
+
+ StubServiceEmitter dynamicActionEmitter = new StubServiceEmitter();
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(10);
+
+ LagBasedAutoScaler autoScaler = new LagBasedAutoScaler(
+ supervisor,
+ DATASOURCE,
+ mapper.convertValue(
+ getScaleOutProperties(2),
+ LagBasedAutoScalerConfig.class
+ ),
+ spec,
+ dynamicActionEmitter
+ );
+
+ supervisor.addTaskGroupToPendingCompletionTaskGroup(
+ 0,
+ ImmutableMap.of("0", "0"),
+ null,
+ null,
+ Set.of("dummyTask"),
+ Collections.emptySet()
+ );
+
+ supervisor.start();
+ autoScaler.start();
+
+ supervisor.runInternal();
+ Thread.sleep(1000); // ensure a dynamic allocation notice completes
+
+ Assert.assertEquals(1, supervisor.getIoConfig().getTaskCount().intValue());
+ Assert.assertTrue(
+ dynamicActionEmitter
+
.getMetricEvents(SeekableStreamSupervisor.AUTOSCALER_REQUIRED_TASKS_METRIC)
+ .stream()
+ .map(metric ->
metric.getUserDims().get(SeekableStreamSupervisor.AUTOSCALER_SKIP_REASON_DIMENSION))
+ .filter(Objects::nonNull)
+ .anyMatch("There are tasks pending completion"::equals)
+ );
+
+
emitter.verifyNotEmitted(SeekableStreamSupervisor.AUTOSCALER_SCALING_TIME_METRIC);
+ autoScaler.reset();
+ autoScaler.stop();
+ }
+
+ @Test
+ public void test_dynamicAllocationNotice_skips_whenSupervisorSuspended()
throws InterruptedException
+ {
+ EasyMock.expect(spec.getId()).andReturn(SUPERVISOR).anyTimes();
+
EasyMock.expect(spec.getSupervisorStateManagerConfig()).andReturn(supervisorConfig).anyTimes();
+
+
EasyMock.expect(spec.getDataSchema()).andReturn(getDataSchema()).anyTimes();
+ EasyMock.expect(spec.getIoConfig()).andReturn(getIOConfig(1,
true)).anyTimes();
+
EasyMock.expect(spec.getTuningConfig()).andReturn(getTuningConfig()).anyTimes();
+ EasyMock.expect(spec.getEmitter()).andReturn(emitter).anyTimes();
+ // Suspended → DynamicAllocationTasksNotice should return early and not
scale
+ EasyMock.expect(spec.isSuspended()).andReturn(true).anyTimes();
+ EasyMock.replay(spec);
+
+
EasyMock.expect(ingestionSchema.getIOConfig()).andReturn(seekableStreamSupervisorIOConfig).anyTimes();
+
EasyMock.expect(ingestionSchema.getDataSchema()).andReturn(dataSchema).anyTimes();
+
EasyMock.expect(ingestionSchema.getTuningConfig()).andReturn(seekableStreamSupervisorTuningConfig).anyTimes();
+ EasyMock.replay(ingestionSchema);
+
+
EasyMock.expect(taskMaster.getTaskRunner()).andReturn(Optional.absent()).anyTimes();
+
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.absent()).anyTimes();
+ EasyMock.replay(taskMaster);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(3);
+ LagBasedAutoScaler autoScaler = new LagBasedAutoScaler(
+ supervisor,
+ DATASOURCE,
+ mapper.convertValue(
+ getScaleOutProperties(2),
+ LagBasedAutoScalerConfig.class
+ ),
+ spec,
+ emitter
+ );
+
+ supervisor.start();
+ autoScaler.start();
+ supervisor.runInternal();
+
+ int before = supervisor.getIoConfig().getTaskCount();
+ Thread.sleep(1000);
+ int after = supervisor.getIoConfig().getTaskCount();
+ // No scaling expected because supervisor is suspended
+ Assert.assertEquals(before, after);
+
+ autoScaler.reset();
+ autoScaler.stop();
+ }
+
+ @Test
+ public void
test_changeTaskCountInIOConfig_handlesExceptionAndStillUpdatesTaskCount()
throws InterruptedException
+ {
+ EasyMock.expect(spec.getId()).andReturn(SUPERVISOR).anyTimes();
+
EasyMock.expect(spec.getSupervisorStateManagerConfig()).andReturn(supervisorConfig).anyTimes();
+
+
EasyMock.expect(spec.getDataSchema()).andReturn(getDataSchema()).anyTimes();
Review Comment:
## Deprecated method or constructor invocation
Invoking [SeekableStreamSupervisorSpec.getDataSchema](1) should be avoided
because it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10625)
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorScaleDuringTaskRolloverTest.java:
##########
@@ -0,0 +1,612 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.seekablestream.supervisor;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.data.input.impl.ByteEntity;
+import org.apache.druid.data.input.impl.DimensionSchema;
+import org.apache.druid.data.input.impl.JsonInputFormat;
+import org.apache.druid.data.input.impl.StringDimensionSchema;
+import org.apache.druid.data.input.impl.TimestampSpec;
+import org.apache.druid.indexer.granularity.UniformGranularitySpec;
+import org.apache.druid.indexing.common.task.Task;
+import org.apache.druid.indexing.overlord.DataSourceMetadata;
+import org.apache.druid.indexing.overlord.IndexerMetadataStorageCoordinator;
+import org.apache.druid.indexing.overlord.TaskMaster;
+import org.apache.druid.indexing.overlord.TaskStorage;
+import
org.apache.druid.indexing.overlord.supervisor.SupervisorStateManagerConfig;
+import org.apache.druid.indexing.overlord.supervisor.autoscaler.LagStats;
+import
org.apache.druid.indexing.overlord.supervisor.autoscaler.SupervisorTaskAutoScaler;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamDataSourceMetadata;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamEndSequenceNumbers;
+import org.apache.druid.indexing.seekablestream.SeekableStreamIndexTask;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamIndexTaskClientFactory;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamIndexTaskIOConfig;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamIndexTaskTuningConfig;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamStartSequenceNumbers;
+import org.apache.druid.indexing.seekablestream.common.OrderedSequenceNumber;
+import org.apache.druid.indexing.seekablestream.common.RecordSupplier;
+import
org.apache.druid.indexing.seekablestream.supervisor.autoscaler.CostBasedAutoScalerConfig;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.java.util.common.parsers.JSONPathSpec;
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.metrics.StubServiceEmitter;
+import org.apache.druid.query.aggregation.CountAggregatorFactory;
+import org.apache.druid.segment.TestHelper;
+import org.apache.druid.segment.incremental.RowIngestionMetersFactory;
+import org.apache.druid.segment.indexing.DataSchema;
+import org.easymock.EasyMock;
+import org.easymock.EasyMockSupport;
+import org.joda.time.DateTime;
+import org.joda.time.Duration;
+import org.joda.time.Period;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.math.BigInteger;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.concurrent.ScheduledExecutorService;
+
+public class SeekableStreamSupervisorScaleDuringTaskRolloverTest extends
EasyMockSupport
+{
+ private static final ObjectMapper OBJECT_MAPPER =
TestHelper.makeJsonMapper();
+ private static final String STREAM = "stream";
+ private static final String DATASOURCE = "testDS";
+ private static final String SUPERVISOR = "supervisor";
+ private static final int DEFAULT_TASK_COUNT = 10;
+
+ private TaskStorage taskStorage;
+ private TaskMaster taskMaster;
+ private IndexerMetadataStorageCoordinator indexerMetadataStorageCoordinator;
+ private ServiceEmitter emitter;
+ private RowIngestionMetersFactory rowIngestionMetersFactory;
+ private SeekableStreamIndexTaskClientFactory taskClientFactory;
+ private SeekableStreamSupervisorSpec spec;
+ private SupervisorStateManagerConfig supervisorConfig;
+
+ @Before
+ public void setUp()
+ {
+ taskStorage = EasyMock.mock(TaskStorage.class);
+ taskMaster = EasyMock.mock(TaskMaster.class);
+ indexerMetadataStorageCoordinator =
EasyMock.mock(IndexerMetadataStorageCoordinator.class);
+ emitter = new StubServiceEmitter();
+ rowIngestionMetersFactory = EasyMock.mock(RowIngestionMetersFactory.class);
+ taskClientFactory =
EasyMock.mock(SeekableStreamIndexTaskClientFactory.class);
+ spec = EasyMock.mock(SeekableStreamSupervisorSpec.class);
+ supervisorConfig = new SupervisorStateManagerConfig();
+
+ // Common taskMaster setup - used by all tests
+
EasyMock.expect(taskMaster.getTaskRunner()).andReturn(Optional.absent()).anyTimes();
+
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.absent()).anyTimes();
+ EasyMock.replay(taskMaster);
+ }
+
+ @Test
+ public void test_maybeScaleDuringTaskRollover_noAutoScaler_doesNotScale()
+ {
+ // Given
+ setupSpecExpectations(getIOConfigWithoutAutoScaler(5));
+
EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject())).andReturn(null).anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(10);
+ supervisor.start();
+
+ int beforeTaskCount = supervisor.getIoConfig().getTaskCount();
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should not change when taskAutoScaler is null",
+ beforeTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ @Test
+ public void
test_maybeScaleDuringTaskRollover_rolloverCountNonPositive_doesNotScale()
+ {
+ // Given
+ setupSpecExpectations(getIOConfigWithCostBasedAutoScaler());
+ EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject()))
+ .andReturn(createMockAutoScaler(-1))
+ .anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(100);
+ supervisor.start();
+ supervisor.createAutoscaler(spec);
+
+ int beforeTaskCount = supervisor.getIoConfig().getTaskCount();
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should not change when rolloverTaskCount <= 0",
+ beforeTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ @Test
+ public void
test_maybeScaleDuringTaskRollover_rolloverCountPositive_performsScaling()
+ {
+ // Given
+ final int targetTaskCount = 5;
+
+ setupSpecExpectations(getIOConfigWithCostBasedAutoScaler());
+ EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject()))
+ .andReturn(createMockAutoScaler(targetTaskCount))
+ .anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(100);
+ supervisor.start();
+ supervisor.createAutoscaler(spec);
+
+ Assert.assertEquals(1, (int) supervisor.getIoConfig().getTaskCount());
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should be updated to " + targetTaskCount + " when
rolloverTaskCount > 0",
+ targetTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ @Test
+ public void
test_maybeScaleDuringTaskRollover_rolloverCountZero_doesNotScale()
+ {
+ // Given
+ setupSpecExpectations(getIOConfigWithCostBasedAutoScaler());
+ EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject()))
+ .andReturn(createMockAutoScaler(0))
+ .anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(100);
+ supervisor.start();
+ supervisor.createAutoscaler(spec);
+
+ int beforeTaskCount = supervisor.getIoConfig().getTaskCount();
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should not change when rolloverTaskCount is 0",
+ beforeTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ // Helper methods for test setup
+
+ /**
+ * Sets up common spec expectations. Call EasyMock.replay(spec) after this
and any additional expectations.
+ */
+ private void setupSpecExpectations(SeekableStreamSupervisorIOConfig ioConfig)
+ {
+ EasyMock.expect(spec.getId()).andReturn(SUPERVISOR).anyTimes();
+
EasyMock.expect(spec.getSupervisorStateManagerConfig()).andReturn(supervisorConfig).anyTimes();
+
EasyMock.expect(spec.getDataSchema()).andReturn(getDataSchema()).anyTimes();
+ EasyMock.expect(spec.getIoConfig()).andReturn(ioConfig).anyTimes();
+
EasyMock.expect(spec.getTuningConfig()).andReturn(getTuningConfig()).anyTimes();
+ EasyMock.expect(spec.getEmitter()).andReturn(emitter).anyTimes();
+ EasyMock.expect(spec.isSuspended()).andReturn(false).anyTimes();
+ }
+
+ /**
+ * Creates a mock autoscaler that returns the specified rollover count.
+ */
+ private static SupervisorTaskAutoScaler createMockAutoScaler(int
rolloverCount)
+ {
+ return new SupervisorTaskAutoScaler()
+ {
+ @Override
+ public void start()
+ {
+ }
+
+ @Override
+ public void stop()
+ {
+ }
+
+ @Override
+ public void reset()
+ {
+ }
+
+ @Override
+ public int computeTaskCountForRollover()
+ {
+ return rolloverCount;
+ }
+ };
+ }
+
+ // Helper methods for config creation
+
+ private static CostBasedAutoScalerConfig getCostBasedAutoScalerConfig()
+ {
+ return CostBasedAutoScalerConfig.builder()
+ .taskCountMax(100)
+ .taskCountMin(1)
+ .enableTaskAutoScaler(true)
+ .lagWeight(0.3)
+ .idleWeight(0.7)
+ .scaleActionPeriodMillis(100)
+ .build();
+ }
+
+ private SeekableStreamSupervisorIOConfig getIOConfigWithCostBasedAutoScaler()
+ {
+ return createIOConfig(DEFAULT_TASK_COUNT, getCostBasedAutoScalerConfig());
+ }
+
+ private SeekableStreamSupervisorIOConfig getIOConfigWithoutAutoScaler(int
taskCount)
+ {
+ return createIOConfig(taskCount, null);
+ }
+
+ private SeekableStreamSupervisorIOConfig createIOConfig(int taskCount,
CostBasedAutoScalerConfig autoScalerConfig)
+ {
+ return new SeekableStreamSupervisorIOConfig(
+ STREAM,
+ new JsonInputFormat(new JSONPathSpec(true, ImmutableList.of()),
ImmutableMap.of(), false, false, false),
+ 1,
+ taskCount,
+ new Period("PT1H"),
+ new Period("P1D"),
+ new Period("PT30S"),
+ false,
+ new Period("PT30M"),
+ null,
+ null,
+ autoScalerConfig,
+ LagAggregator.DEFAULT,
+ null,
+ null,
+ null
+ )
+ {
+ };
+ }
+
+ private static DataSchema getDataSchema()
+ {
+ List<DimensionSchema> dimensions = new ArrayList<>();
+ dimensions.add(StringDimensionSchema.create("dim1"));
+ dimensions.add(StringDimensionSchema.create("dim2"));
+
+ return DataSchema.builder()
+ .withDataSource(DATASOURCE)
+ .withTimestamp(new TimestampSpec("timestamp", "iso",
null))
+ .withDimensions(dimensions)
+ .withAggregators(new CountAggregatorFactory("rows"))
+ .withGranularity(
+ new UniformGranularitySpec(
+ Granularities.HOUR,
+ Granularities.NONE,
+ ImmutableList.of()
+ )
+ )
+ .build();
+ }
+
+ private static SeekableStreamSupervisorTuningConfig getTuningConfig()
+ {
+ return new SeekableStreamSupervisorTuningConfig()
+ {
+ @Override
+ public Integer getWorkerThreads()
+ {
+ return 1;
+ }
+
+ @Override
+ public Long getChatRetries()
+ {
+ return 1L;
+ }
+
+ @Override
+ public Duration getHttpTimeout()
+ {
+ return new Period("PT1M").toStandardDuration();
+ }
+
+ @Override
+ public Duration getShutdownTimeout()
+ {
+ return new Period("PT1S").toStandardDuration();
+ }
+
+ @Override
+ public Duration getRepartitionTransitionDuration()
+ {
+ return new Period("PT2M").toStandardDuration();
+ }
+
+ @Override
+ public Duration getOffsetFetchPeriod()
+ {
+ return new Period("PT5M").toStandardDuration();
+ }
+
+ @Override
+ public SeekableStreamIndexTaskTuningConfig convertToTaskTuningConfig()
+ {
+ return new SeekableStreamIndexTaskTuningConfig(null, null, null, null,
null, null, null, null, null, null,
+ null, null, null, null, null, null, null, null, null, null,
+ null, null, null
+ )
+ {
+ @Override
+ public SeekableStreamIndexTaskTuningConfig
withBasePersistDirectory(File dir)
+ {
+ return null;
+ }
+
+ @Override
+ public String toString()
+ {
+ return null;
+ }
+ };
+ }
+ };
+ }
+
+ // Inner test classes
+
+ private abstract class BaseTestSeekableStreamSupervisor extends
SeekableStreamSupervisor<String, String, ByteEntity>
+ {
+ private BaseTestSeekableStreamSupervisor()
+ {
+ super(
+ "testSupervisorId",
+ taskStorage,
+ taskMaster,
+ indexerMetadataStorageCoordinator,
+ taskClientFactory,
+ OBJECT_MAPPER,
+ spec,
+ rowIngestionMetersFactory,
+ false
+ );
+ }
+
+ @Override
+ protected String baseTaskName()
+ {
+ return "test";
+ }
+
+ @Override
+ protected void updatePartitionLagFromStream()
+ {
+ }
+
+ @Nullable
+ @Override
+ protected Map<String, Long> getPartitionRecordLag()
+ {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ protected Map<String, Long> getPartitionTimeLag()
+ {
+ return null;
+ }
+
+ @Override
+ protected SeekableStreamIndexTaskIOConfig createTaskIoConfig(
+ int groupId,
+ Map<String, String> startPartitions,
+ Map<String, String> endPartitions,
+ String baseSequenceName,
+ DateTime minimumMessageTime,
+ DateTime maximumMessageTime,
+ Set<String> exclusiveStartSequenceNumberPartitions,
+ SeekableStreamSupervisorIOConfig ioConfig
+ )
+ {
+ return new SeekableStreamIndexTaskIOConfig<>(
+ groupId,
+ baseSequenceName,
+ new SeekableStreamStartSequenceNumbers<>(STREAM, startPartitions,
exclusiveStartSequenceNumberPartitions),
+ new SeekableStreamEndSequenceNumbers<>(STREAM, endPartitions),
+ true,
+ minimumMessageTime,
+ maximumMessageTime,
+ ioConfig.getInputFormat(),
+ ioConfig.getTaskDuration().getStandardMinutes()
+ )
+ {
+ };
+ }
+
+ @Override
+ protected List<SeekableStreamIndexTask<String, String, ByteEntity>>
createIndexTasks(
+ int replicas,
+ String baseSequenceName,
+ ObjectMapper sortingMapper,
+ TreeMap<Integer, Map<String, String>> sequenceOffsets,
+ SeekableStreamIndexTaskIOConfig taskIoConfig,
+ SeekableStreamIndexTaskTuningConfig taskTuningConfig,
+ RowIngestionMetersFactory rowIngestionMetersFactory
+ )
+ {
+ return null;
+ }
+
+ @Override
+ protected int getTaskGroupIdForPartition(String partition)
+ {
+ return 0;
+ }
+
+ @Override
+ protected boolean checkSourceMetadataMatch(DataSourceMetadata metadata)
+ {
+ return true;
+ }
+
+ @Override
+ protected boolean doesTaskMatchSupervisor(Task task)
+ {
+ return true;
+ }
+
+ @Override
+ protected SeekableStreamDataSourceMetadata<String, String>
createDataSourceMetaDataForReset(
+ String stream,
+ Map<String, String> map
+ )
+ {
+ return null;
+ }
+
+ @Override
+ protected OrderedSequenceNumber<String> makeSequenceNumber(String seq,
boolean isExclusive)
+ {
+ return new OrderedSequenceNumber<>(seq, isExclusive)
Review Comment:
## Inconsistent compareTo
This class declares [compareTo](1) but inherits equals; the two could be
inconsistent.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10622)
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorScaleDuringTaskRolloverTest.java:
##########
@@ -0,0 +1,612 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.seekablestream.supervisor;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.data.input.impl.ByteEntity;
+import org.apache.druid.data.input.impl.DimensionSchema;
+import org.apache.druid.data.input.impl.JsonInputFormat;
+import org.apache.druid.data.input.impl.StringDimensionSchema;
+import org.apache.druid.data.input.impl.TimestampSpec;
+import org.apache.druid.indexer.granularity.UniformGranularitySpec;
+import org.apache.druid.indexing.common.task.Task;
+import org.apache.druid.indexing.overlord.DataSourceMetadata;
+import org.apache.druid.indexing.overlord.IndexerMetadataStorageCoordinator;
+import org.apache.druid.indexing.overlord.TaskMaster;
+import org.apache.druid.indexing.overlord.TaskStorage;
+import
org.apache.druid.indexing.overlord.supervisor.SupervisorStateManagerConfig;
+import org.apache.druid.indexing.overlord.supervisor.autoscaler.LagStats;
+import
org.apache.druid.indexing.overlord.supervisor.autoscaler.SupervisorTaskAutoScaler;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamDataSourceMetadata;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamEndSequenceNumbers;
+import org.apache.druid.indexing.seekablestream.SeekableStreamIndexTask;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamIndexTaskClientFactory;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamIndexTaskIOConfig;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamIndexTaskTuningConfig;
+import
org.apache.druid.indexing.seekablestream.SeekableStreamStartSequenceNumbers;
+import org.apache.druid.indexing.seekablestream.common.OrderedSequenceNumber;
+import org.apache.druid.indexing.seekablestream.common.RecordSupplier;
+import
org.apache.druid.indexing.seekablestream.supervisor.autoscaler.CostBasedAutoScalerConfig;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.java.util.common.parsers.JSONPathSpec;
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.metrics.StubServiceEmitter;
+import org.apache.druid.query.aggregation.CountAggregatorFactory;
+import org.apache.druid.segment.TestHelper;
+import org.apache.druid.segment.incremental.RowIngestionMetersFactory;
+import org.apache.druid.segment.indexing.DataSchema;
+import org.easymock.EasyMock;
+import org.easymock.EasyMockSupport;
+import org.joda.time.DateTime;
+import org.joda.time.Duration;
+import org.joda.time.Period;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.math.BigInteger;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.concurrent.ScheduledExecutorService;
+
+public class SeekableStreamSupervisorScaleDuringTaskRolloverTest extends
EasyMockSupport
+{
+ private static final ObjectMapper OBJECT_MAPPER =
TestHelper.makeJsonMapper();
+ private static final String STREAM = "stream";
+ private static final String DATASOURCE = "testDS";
+ private static final String SUPERVISOR = "supervisor";
+ private static final int DEFAULT_TASK_COUNT = 10;
+
+ private TaskStorage taskStorage;
+ private TaskMaster taskMaster;
+ private IndexerMetadataStorageCoordinator indexerMetadataStorageCoordinator;
+ private ServiceEmitter emitter;
+ private RowIngestionMetersFactory rowIngestionMetersFactory;
+ private SeekableStreamIndexTaskClientFactory taskClientFactory;
+ private SeekableStreamSupervisorSpec spec;
+ private SupervisorStateManagerConfig supervisorConfig;
+
+ @Before
+ public void setUp()
+ {
+ taskStorage = EasyMock.mock(TaskStorage.class);
+ taskMaster = EasyMock.mock(TaskMaster.class);
+ indexerMetadataStorageCoordinator =
EasyMock.mock(IndexerMetadataStorageCoordinator.class);
+ emitter = new StubServiceEmitter();
+ rowIngestionMetersFactory = EasyMock.mock(RowIngestionMetersFactory.class);
+ taskClientFactory =
EasyMock.mock(SeekableStreamIndexTaskClientFactory.class);
+ spec = EasyMock.mock(SeekableStreamSupervisorSpec.class);
+ supervisorConfig = new SupervisorStateManagerConfig();
+
+ // Common taskMaster setup - used by all tests
+
EasyMock.expect(taskMaster.getTaskRunner()).andReturn(Optional.absent()).anyTimes();
+
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.absent()).anyTimes();
+ EasyMock.replay(taskMaster);
+ }
+
+ @Test
+ public void test_maybeScaleDuringTaskRollover_noAutoScaler_doesNotScale()
+ {
+ // Given
+ setupSpecExpectations(getIOConfigWithoutAutoScaler(5));
+
EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject())).andReturn(null).anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(10);
+ supervisor.start();
+
+ int beforeTaskCount = supervisor.getIoConfig().getTaskCount();
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should not change when taskAutoScaler is null",
+ beforeTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ @Test
+ public void
test_maybeScaleDuringTaskRollover_rolloverCountNonPositive_doesNotScale()
+ {
+ // Given
+ setupSpecExpectations(getIOConfigWithCostBasedAutoScaler());
+ EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject()))
+ .andReturn(createMockAutoScaler(-1))
+ .anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(100);
+ supervisor.start();
+ supervisor.createAutoscaler(spec);
+
+ int beforeTaskCount = supervisor.getIoConfig().getTaskCount();
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should not change when rolloverTaskCount <= 0",
+ beforeTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ @Test
+ public void
test_maybeScaleDuringTaskRollover_rolloverCountPositive_performsScaling()
+ {
+ // Given
+ final int targetTaskCount = 5;
+
+ setupSpecExpectations(getIOConfigWithCostBasedAutoScaler());
+ EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject()))
+ .andReturn(createMockAutoScaler(targetTaskCount))
+ .anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(100);
+ supervisor.start();
+ supervisor.createAutoscaler(spec);
+
+ Assert.assertEquals(1, (int) supervisor.getIoConfig().getTaskCount());
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should be updated to " + targetTaskCount + " when
rolloverTaskCount > 0",
+ targetTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ @Test
+ public void
test_maybeScaleDuringTaskRollover_rolloverCountZero_doesNotScale()
+ {
+ // Given
+ setupSpecExpectations(getIOConfigWithCostBasedAutoScaler());
+ EasyMock.expect(spec.createAutoscaler(EasyMock.anyObject()))
+ .andReturn(createMockAutoScaler(0))
+ .anyTimes();
+ EasyMock.replay(spec);
+
+ TestSeekableStreamSupervisor supervisor = new
TestSeekableStreamSupervisor(100);
+ supervisor.start();
+ supervisor.createAutoscaler(spec);
+
+ int beforeTaskCount = supervisor.getIoConfig().getTaskCount();
+
+ // When
+ supervisor.maybeScaleDuringTaskRollover();
+
+ // Then
+ Assert.assertEquals(
+ "Task count should not change when rolloverTaskCount is 0",
+ beforeTaskCount,
+ (int) supervisor.getIoConfig().getTaskCount()
+ );
+ }
+
+ // Helper methods for test setup
+
+ /**
+ * Sets up common spec expectations. Call EasyMock.replay(spec) after this
and any additional expectations.
+ */
+ private void setupSpecExpectations(SeekableStreamSupervisorIOConfig ioConfig)
+ {
+ EasyMock.expect(spec.getId()).andReturn(SUPERVISOR).anyTimes();
+
EasyMock.expect(spec.getSupervisorStateManagerConfig()).andReturn(supervisorConfig).anyTimes();
+
EasyMock.expect(spec.getDataSchema()).andReturn(getDataSchema()).anyTimes();
Review Comment:
## Deprecated method or constructor invocation
Invoking [SeekableStreamSupervisorSpec.getDataSchema](1) should be avoided
because it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10623)
--
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]
