kfaraz commented on code in PR #18108:
URL: https://github.com/apache/druid/pull/18108#discussion_r2139558012
##########
server/src/test/java/org/apache/druid/metadata/IndexerSqlMetadataStorageCoordinatorTestBase.java:
##########
@@ -332,14 +330,15 @@ protected List<DataSegment>
createAndGetUsedYearSegments(final int startYear, fi
for (int year = startYear; year < endYear; year++) {
segments.add(createSegment(
- Intervals.of("%d/%d", year, year + 1),
- "version",
- new LinearShardSpec(0))
+ Intervals.of("%d/%d", year, year + 1),
+ "version",
+ new LinearShardSpec(0)
+ )
);
}
final Set<DataSegment> segmentsSet = new HashSet<>(segments);
- final Set<DataSegment> committedSegments =
coordinator.commitSegments(segmentsSet, new SegmentSchemaMapping(
- CentralizedDatasourceSchemaConfig.SCHEMA_VERSION));
+ final Set<DataSegment> committedSegments = coordinator.commitSegments(
+ segmentsSet, null);
Review Comment:
```suggestion
final Set<DataSegment> committedSegments =
coordinator.commitSegments(segmentsSet, null);
```
##########
server/src/test/java/org/apache/druid/metadata/TestDerbyConnector.java:
##########
@@ -144,7 +181,11 @@ public String getConnectURI()
@Override
protected void before()
{
- connector = new
TestDerbyConnector(Suppliers.ofInstance(connectorConfig), dbTables,
centralizedDatasourceSchemaConfig);
+ connector = new TestDerbyConnector(
+ connectorConfig,
+ dbTables,
+ centralizedDatasourceSchemaConfig
+ );
connector.getDBI().open().close(); // create db
Review Comment:
Use `connector.createDatabase()` instead.
##########
server/src/test/java/org/apache/druid/metadata/TestDerbyConnector.java:
##########
@@ -49,33 +49,55 @@
public class TestDerbyConnector extends DerbyConnector
{
private final String jdbcUri;
+ private final MetadataStorageTablesConfig dbTables;
public TestDerbyConnector(
- Supplier<MetadataStorageConnectorConfig> config,
- Supplier<MetadataStorageTablesConfig> dbTables,
+ MetadataStorageConnectorConfig config,
+ MetadataStorageTablesConfig dbTables,
Review Comment:
Please rename these to `connectorConfig` and `tablesConfig` respectively,
same in other constructors.
##########
processing/src/main/java/org/apache/druid/java/util/common/Intervals.java:
##########
@@ -53,6 +54,11 @@ public static Interval of(String format, Object...
formatArgs)
return of(StringUtils.format(format, formatArgs));
}
+ public static Interval of(Object... formatArgs)
+ {
+ return of(DEFAULT_INTERVAL_FORMAT, formatArgs);
+ }
Review Comment:
This doesn't seem right.
It can cause ambiguity with `Intervals.of()` and is also in general an
ambiguous method.
##########
benchmarks/src/test/java/org/apache/druid/benchmark/indexing/SqlSegmentsMetadataQueryBenchmark.java:
##########
@@ -0,0 +1,139 @@
+package org.apache.druid.benchmark.indexing;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.druid.java.util.common.DateTimes;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.java.util.common.parsers.CloseableIterator;
+import org.apache.druid.metadata.IndexerSqlMetadataStorageCoordinatorTestBase;
+import org.apache.druid.metadata.MetadataStorageTablesConfig;
+import org.apache.druid.metadata.SqlSegmentsMetadataQuery;
+import org.apache.druid.metadata.TestDerbyConnector;
+import org.apache.druid.segment.TestDataSource;
+import org.apache.druid.segment.TestHelper;
+import org.apache.druid.server.coordinator.CreateDataSegments;
+import org.apache.druid.timeline.DataSegment;
+import org.joda.time.DateTime;
+import org.joda.time.Interval;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.TearDown;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+@State(Scope.Benchmark)
+@Fork(value = 1)
+@Warmup(iterations = 1, time = 1)
+@Measurement(iterations = 20, time = 2)
+@BenchmarkMode({Mode.AverageTime})
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+public class SqlSegmentsMetadataQueryBenchmark
+{
+
+ private static final DateTime JAN_1 = DateTimes.of("2025-01-01");
+ private static final String V1 = JAN_1.toString();
+ private static final List<DataSegment> WIKI_SEGMENTS_1000X100D
+ = CreateDataSegments.ofDatasource(TestDataSource.WIKI)
+ .forIntervals(100, Granularities.DAY)
+ .withNumPartitions(1000)
+ .startingAt(JAN_1)
+ .withVersion(V1)
+ .eachOfSizeInMb(500);
+
+ private TestDerbyConnector derbyConnector;
+
+ @Setup(Level.Trial)
+ public void setup() throws Exception
+ {
+ this.derbyConnector = new TestDerbyConnector();
+ derbyConnector.createDatabase();
+ derbyConnector.createSegmentTable();
+ insertSegments(WIKI_SEGMENTS_1000X100D.toArray(new DataSegment[0]));
+ }
+
+ @TearDown(Level.Trial)
+ public void tearDown() throws Exception
+ {
+ derbyConnector.tearDown();
+ }
+
+ @Benchmark
+ public void benchmarkRetrieveUsedSegments_returnAllSegments(Blackhole
blackhole)
+ {
+ final Interval queryInterval = Intervals.of(JAN_1, JAN_1.plusDays(3));
+ blackhole.consume(readAsSet(q ->
q.retrieveUsedSegments(TestDataSource.WIKI, List.of(queryInterval))));
+ }
+
+ @Benchmark
+ public void benchmarkRetrieveUsedSegments_returnEmpty(Blackhole blackhole)
+ {
+ final Interval queryInterval = Intervals.of(JAN_1.plusDays(-2),
JAN_1.plusDays(-1));
Review Comment:
Use `.minusDays(2)` instead of `plusDays(-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]
