kfaraz commented on code in PR #16623:
URL: https://github.com/apache/druid/pull/16623#discussion_r1644352693
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v0",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForExactIntervalAndVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v1",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForDifferentInterval = createSegment(
+ Intervals.of("2023/2024"),
Review Comment:
Rather than a disjoint interval, a better test would be to verify that a
segment in an overlapping (but not identical) interval is not returned.
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
Review Comment:
```suggestion
final DataSegment unusedSegmentMay2024V0 = createSegment(
```
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v0",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForExactIntervalAndVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v1",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForDifferentInterval = createSegment(
+ Intervals.of("2023/2024"),
+ "v1",
+ new NumberedShardSpec(0, 0)
+ );
+ coordinator.commitSegments(
+ ImmutableSet.of(
+ unusedForDifferentVersion,
+ unusedSegmentForDifferentInterval,
+ unusedSegmentForExactIntervalAndVersion
+ ),
+ null
+ );
+ coordinator.markSegmentsAsUnusedWithinInterval(DS.WIKI,
Intervals.ETERNITY);
+
+ DataSegment usedSegmentForExactIntervalAndVersion = createSegment(
+ Intervals.of("2024/2025"),
Review Comment:
```suggestion
Intervals.of("2024-05/P1M"),
```
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v0",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForExactIntervalAndVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v1",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForDifferentInterval = createSegment(
+ Intervals.of("2023/2024"),
+ "v1",
+ new NumberedShardSpec(0, 0)
+ );
+ coordinator.commitSegments(
+ ImmutableSet.of(
+ unusedForDifferentVersion,
+ unusedSegmentForDifferentInterval,
+ unusedSegmentForExactIntervalAndVersion
+ ),
+ null
+ );
+ coordinator.markSegmentsAsUnusedWithinInterval(DS.WIKI,
Intervals.ETERNITY);
+
+ DataSegment usedSegmentForExactIntervalAndVersion = createSegment(
Review Comment:
```suggestion
final DataSegment usedSegmentMay2024V1 = createSegment(
```
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
Review Comment:
Nit: use an interval which is easier to use in a name. You may even assign
this interval value to a field named `Interval may2024` so that you can reuse
it in multiple places.
```suggestion
Intervals.of("2024-05/P1M"),
```
##########
server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java:
##########
@@ -249,6 +249,44 @@ public List<Pair<DataSegment, String>>
retrieveUsedSegmentsAndCreatedDates(Strin
);
}
+ List<String> retrieveUnusedSegmentIdsForExactIntervalAndVersion(
+ String dataSource,
+ Interval interval,
+ String version
+ )
+ {
+ final String sql = "SELECT id FROM %1$s"
+ + " WHERE used = :used"
+ + " AND dataSource = :dataSource"
+ + " AND version = :version"
+ + " AND start = :start AND %2$send%2$s = :end";
+
+ final List<String> matchingSegments = connector.inReadOnlyTransaction(
+ (handle, status) -> {
+ final Query<Map<String, Object>> query = handle
+ .createQuery(StringUtils.format(
+ sql,
+ dbTables.getSegmentsTable(),
+ connector.getQuoteString()
+ ))
+ .setFetchSize(connector.getStreamingFetchSize())
+ .bind("used", false)
+ .bind("dataSource", dataSource)
+ .bind("version", version)
+ .bind("start", interval.getStart().toString())
+ .bind("end", interval.getEnd().toString());
+
+ try (final ResultIterator<String> iterator = query.map((index, r,
ctx) -> r.getString(1)).iterator()) {
+ return ImmutableList.copyOf(iterator);
+ }
+ }
+ );
+
+ log.debug("Found [%,d] unused segments for datasource[%s] for interval[%s]
and version[%s].",
+ matchingSegments.size(), dataSource, interval, version);
Review Comment:
Nit:
```suggestion
log.debug(
"Found [%,d] unused segments for datasource[%s], interval[%s] and
version[%s].",
matchingSegments.size(), dataSource, interval, version
);
```
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v0",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForExactIntervalAndVersion = createSegment(
Review Comment:
```suggestion
final DataSegment unusedSegmentMay2024V1 = createSegment(
```
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v0",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForExactIntervalAndVersion = createSegment(
+ Intervals.of("2024/2025"),
Review Comment:
```suggestion
Intervals.of("2024-05/P1M"),
```
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v0",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForExactIntervalAndVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v1",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForDifferentInterval = createSegment(
Review Comment:
```suggestion
final DataSegment unusedSegmentYear2024V1 = createSegment(
```
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws
IOException
);
Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(),
true));
}
+
+ @Test
+ public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws
Exception
+ {
+ DataSegment unusedForDifferentVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v0",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForExactIntervalAndVersion = createSegment(
+ Intervals.of("2024/2025"),
+ "v1",
+ new NumberedShardSpec(0, 0)
+ );
+ DataSegment unusedSegmentForDifferentInterval = createSegment(
+ Intervals.of("2023/2024"),
Review Comment:
```suggestion
Intervals.of("2024/P1Y"),
```
--
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]
