This is an automated email from the ASF dual-hosted git repository.
kfaraz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 12116521c88 fix: Limit number of unused segment rows scanned (#19690)
12116521c88 is described below
commit 12116521c8881f6ac5f5f793afbcdeeae5691876
Author: Kashif Faraz <[email protected]>
AuthorDate: Wed Jul 15 22:48:28 2026 +0530
fix: Limit number of unused segment rows scanned (#19690)
* Limit number of unused segment rows scanned in
SqlSegmentsMetadatQuery.retrieveSomeUnusedSegmentIntervals
* Disable checkstyle for text block
* Fix javadocs and method name
---
.../overlord/duty/UnusedSegmentsKiller.java | 2 +-
.../TestIndexerMetadataStorageCoordinator.java | 2 +-
.../IndexerMetadataStorageCoordinator.java | 18 +++++++--
.../IndexerSQLMetadataStorageCoordinator.java | 4 +-
.../druid/metadata/SqlSegmentsMetadataQuery.java | 43 +++++++++++++++++-----
.../IndexerSQLMetadataStorageCoordinatorTest.java | 10 ++---
6 files changed, 56 insertions(+), 23 deletions(-)
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/UnusedSegmentsKiller.java
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/UnusedSegmentsKiller.java
index b582ae80acd..daaef7d6c03 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/UnusedSegmentsKiller.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/UnusedSegmentsKiller.java
@@ -249,7 +249,7 @@ public class UnusedSegmentsKiller implements OverlordDuty
final Map<String, Integer> dataSourceToIntervalCounts = new HashMap<>();
for (String dataSource : dataSources) {
- storageCoordinator.retrieveUnusedSegmentIntervals(dataSource,
MAX_INTERVALS_TO_KILL_IN_DATASOURCE).forEach(
+ storageCoordinator.retrieveSomeUnusedSegmentIntervals(dataSource,
MAX_INTERVALS_TO_KILL_IN_DATASOURCE).forEach(
interval -> {
dataSourceToIntervalCounts.merge(dataSource, 1, Integer::sum);
killQueue.offer(new KillCandidate(dataSource, interval));
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/test/TestIndexerMetadataStorageCoordinator.java
b/indexing-service/src/test/java/org/apache/druid/indexing/test/TestIndexerMetadataStorageCoordinator.java
index 00cddb943c7..dbc5a5def5c 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/test/TestIndexerMetadataStorageCoordinator.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/test/TestIndexerMetadataStorageCoordinator.java
@@ -70,7 +70,7 @@ public class TestIndexerMetadataStorageCoordinator implements
IndexerMetadataSto
}
@Override
- public List<Interval> retrieveUnusedSegmentIntervals(String dataSource, int
limit)
+ public List<Interval> retrieveSomeUnusedSegmentIntervals(String dataSource,
int limit)
{
return List.of();
}
diff --git
a/server/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageCoordinator.java
b/server/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageCoordinator.java
index 5ac25f6ad0b..db9b9835ef5 100644
---
a/server/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageCoordinator.java
+++
b/server/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageCoordinator.java
@@ -600,12 +600,22 @@ public interface IndexerMetadataStorageCoordinator
/**
* Retrieves intervals of the specified datasource that contain any unused
segments.
- * There is no guarantee on the order of intervals in the list or on whether
- * the limited list contains the earliest or latest intervals of the
datasource.
+ * <p>
+ * This method ensures that if there is any unused segment for the
datasource,
+ * the returned list is not empty. However, it does NOT guarantee that:
+ * <ul>
+ * <li>the intervals in the result would be ordered</li>
+ * <li>the result would contain the earliest or latest intervals for this
datasource</li>
+ * <li>it would scan all unused segments for this datasource. So, the result
+ * may contain less than {@code limit} entries even when there are more
distinct
+ * unused segment intervals in the metadata store for this datasource.</li>
+ * </ul>
*
- * @return Unsorted list of unused segment intervals containing upto {@code
limit} entries.
+ * @return List of distinct unused segment intervals for the specified
datasource
+ * containing at least 1 entry if there is any unused segment for the
datasource,
+ * upto a maximum of {@code limit} entries.
*/
- List<Interval> retrieveUnusedSegmentIntervals(String dataSource, int limit);
+ List<Interval> retrieveSomeUnusedSegmentIntervals(String dataSource, int
limit);
/**
* Returns the number of segment entries in the database whose state was
changed as the result of this call (that is,
diff --git
a/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
b/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
index 3aa06b8e371..1317fd339a6 100644
---
a/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
+++
b/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
@@ -164,10 +164,10 @@ public class IndexerSQLMetadataStorageCoordinator
implements IndexerMetadataStor
}
@Override
- public List<Interval> retrieveUnusedSegmentIntervals(String dataSource, int
limit)
+ public List<Interval> retrieveSomeUnusedSegmentIntervals(String dataSource,
int limit)
{
return inReadOnlyTransaction(
- sql -> sql.retrieveUnusedSegmentIntervals(dataSource, limit)
+ sql -> sql.retrieveSomeUnusedSegmentIntervals(dataSource, limit)
);
}
diff --git
a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java
b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java
index 6257a11b58a..559d3009436 100644
---
a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java
+++
b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java
@@ -1065,20 +1065,43 @@ public class SqlSegmentsMetadataQuery
}
/**
- * Gets unused segment intervals for the specified datasource. There is no
- * guarantee on the order of intervals in the list or on whether the limited
- * list contains the earliest or latest intervals present in the datasource.
+ * Retrieves intervals containing unused segments for the specified
datasource.
+ * <p>
+ * This method ensures that if there is any unused segment for the
datasource,
+ * the returned list is not empty. However, it does NOT guarantee that:
+ * <ul>
+ * <li>the intervals in the result would be ordered</li>
+ * <li>the result would contain the earliest or latest intervals for this
datasource</li>
+ * <li>it would scan all unused segments for this datasource. So, the result
+ * may contain less than {@code limit} entries even when there are more
distinct
+ * unused segment intervals in the metadata store for this datasource.</li>
+ * </ul>
*
- * @return List of unused segment intervals containing upto {@code limit}
interval entries.
+ * @return List of distinct unused segment intervals for the specified
datasource
+ * containing at least 1 entry if there is any unused segment for the
datasource,
+ * upto a maximum of {@code limit} entries.
*/
- public List<Interval> retrieveUnusedSegmentIntervals(String dataSource, int
limit)
+ public List<Interval> retrieveSomeUnusedSegmentIntervals(String dataSource,
int limit)
{
final String sql = StringUtils.format(
- "SELECT start, %2$send%2$s FROM %1$s"
- + " WHERE dataSource = :dataSource AND used = false"
- + " GROUP BY %2$send%2$s, start"
- + " %3$s",
- dbTables.getSegmentsTable(), connector.getQuoteString(),
connector.limitClause(limit)
+ // Disable checkstyle to avoid argumentLineBreaking rule from getting
triggered
+ //CHECKSTYLE.OFF: Regexp
+ """
+ SELECT start, %2$send%2$s
+ FROM (
+ SELECT start, %2$send%2$s
+ FROM %1$s
+ WHERE dataSource = :dataSource AND used = false
+ %3$s
+ ) AS unused
+ GROUP BY %2$send%2$s, start
+ %4$s
+ """,
+ //CHECKSTYLE.ON: Regexp
+ dbTables.getSegmentsTable(),
+ connector.getQuoteString(),
+ connector.limitClause(1_000_000), // limit unused segment rows scanned
to 1M
+ connector.limitClause(limit)
);
final List<Interval> intervals = connector.inReadOnlyTransaction(
diff --git
a/server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java
b/server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java
index 000ba2be3b9..70f4d53cc38 100644
---
a/server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java
+++
b/server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java
@@ -2252,29 +2252,29 @@ public class IndexerSQLMetadataStorageCoordinatorTest
extends IndexerSqlMetadata
}
@Test
- public void testRetrieveUnusedSegmentIntervals()
+ public void testRetrieveSomeUnusedSegmentIntervals()
{
final String dataSource = defaultSegment.getDataSource();
coordinator.commitSegments(Set.of(defaultSegment, defaultSegment3), null);
- Assert.assertTrue(coordinator.retrieveUnusedSegmentIntervals(dataSource,
100).isEmpty());
+
Assert.assertTrue(coordinator.retrieveSomeUnusedSegmentIntervals(dataSource,
100).isEmpty());
markAllSegmentsUnused(Set.of(defaultSegment),
DateTimes.nowUtc().minusHours(1));
Assert.assertEquals(
List.of(defaultSegment.getInterval()),
- coordinator.retrieveUnusedSegmentIntervals(dataSource, 100)
+ coordinator.retrieveSomeUnusedSegmentIntervals(dataSource, 100)
);
markAllSegmentsUnused(Set.of(defaultSegment3),
DateTimes.nowUtc().minusHours(1));
Assert.assertEquals(
Set.of(defaultSegment.getInterval(), defaultSegment3.getInterval()),
- Set.copyOf(coordinator.retrieveUnusedSegmentIntervals(dataSource, 100))
+ Set.copyOf(coordinator.retrieveSomeUnusedSegmentIntervals(dataSource,
100))
);
// Verify retrieve with limit 1 returns only 1 interval
Assert.assertEquals(
1,
- coordinator.retrieveUnusedSegmentIntervals(dataSource, 1).size()
+ coordinator.retrieveSomeUnusedSegmentIntervals(dataSource, 1).size()
);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
