gianm commented on code in PR #19690:
URL: https://github.com/apache/druid/pull/19690#discussion_r3588136242
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java:
##########
@@ -1065,20 +1065,40 @@ public List<Interval> retrieveUnusedSegmentIntervals(
}
/**
- * 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.
+ * Gets unused segment intervals for the specified datasource.
+ * <p>
+ * Note: This method does NOT guarantee that:
Review Comment:
I see why we need to weaken the guarantees. However, I think it still does
guarantee that _if_ there are unused segments, _then_ we will return at least
one interval. This is a useful guarantee that should be mentioned.
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java:
##########
@@ -1065,20 +1065,40 @@ public List<Interval> retrieveUnusedSegmentIntervals(
}
/**
- * 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.
+ * Gets unused segment intervals for the specified datasource.
+ * <p>
+ * Note: This method 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.
*/
public List<Interval> retrieveUnusedSegmentIntervals(String dataSource, int
limit)
Review Comment:
Consider renaming it to `retrieveSomeUnusedSegmentIntervals`, just to
emphasize that not all of them will necessarily be returned.
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java:
##########
@@ -1065,20 +1065,40 @@ public List<Interval> retrieveUnusedSegmentIntervals(
}
/**
- * 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.
+ * Gets unused segment intervals for the specified datasource.
+ * <p>
+ * Note: This method 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.
*/
public List<Interval> retrieveUnusedSegmentIntervals(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)
+ // Text blocks are not supported in the current checkstyle version
Review Comment:
I believe this comment is inaccurate. We use text blocks elsewhere. Maybe
there's some other reason that checkstyle flags this block. The suppression is
for `Regexp` so it could be one of our custom rules.
--
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]
