kfaraz commented on code in PR #19772:
URL: https://github.com/apache/druid/pull/19772#discussion_r3667269989
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java:
##########
@@ -1116,6 +1118,70 @@ public List<Interval>
retrieveSomeUnusedSegmentIntervals(String dataSource, int
return
intervals.stream().filter(Objects::nonNull).collect(Collectors.toList());
}
+ /**
+ * Scans upto {@code maxSegmentsToScan} unused segments which are eligible
for
+ * kill and returns the unique datasource-interval for the segments scanned.
+ * <p>
+ * This method ensures that if there is any unused segment in any datasource
+ * which was updated earlier than {@code maxUpdatedTime}, then the returned
+ * map is not empty. However, it does NOT guarantee that:
+ * <ul>
+ * <li>the candidates in the returned map would be ordered by datasource or
interval</li>
+ * <li>the result would contain {@code limit} entries when there are more
distinct
+ * intervals with eligible unused segments in the metadata store.</li>
+ * </ul>
+ *
+ * @param maxUpdatedTime Unused segments are considered eligible for kill
+ * if they were last updated before this time.
+ * @param maxResultSize Maximum number of candidate intervals to return
+ * across all datasources.
+ * @param maxSegmentsToScan Maximum number of eligible unused segments to
scan
+ * in the metadata store.
+ * @return Map from {@link DatasourceInterval} to the number of unused
segments
+ * eligible for kill.
+ */
+ public Map<DatasourceInterval, Integer> retrieveSomeUnusedSegmentIntervals(
+ DateTime maxUpdatedTime,
+ int maxResultSize,
+ int maxSegmentsToScan
+ )
+ {
+ final String sql = StringUtils.format(
+ // Disable checkstyle to avoid argumentLineBreaking rule from getting
triggered
+ //CHECKSTYLE.OFF: Regexp
+ """
+ SELECT dataSource, start, %2$send%2$s, COUNT(*) AS totalCount
+ FROM (
+ SELECT dataSource, start, %2$send%2$s
+ FROM %1$s
+ WHERE used = false
+ AND (used_status_last_updated IS NULL OR
used_status_last_updated <= :maxUpdatedTime)
+ %3$s
+ ) AS unused
+ GROUP BY dataSource, %2$send%2$s, start
+ %4$s
+ """,
+ //CHECKSTYLE.ON: Regexp
+ dbTables.getSegmentsTable(),
+ connector.getQuoteString(),
+ connector.limitClause(maxSegmentsToScan),
Review Comment:
While this is possible, it is not very likely to happen.
- Firstly, intervals will not remain locked forever, unless there is a lot
of late arriving data in streaming supervisors.
- Secondly, the search query need not pick up the same intervals in the next
iteration.
Also, the eventual goal is to use concurrent locks in embedded kill tasks,
thereby removing the need to skip locked intervals altogether. I will create a
follow up PR for that change.
--
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]
