From abd791332f71417ae5cdfd910f86298d9d5fdf93 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 15:06:49 -0700 Subject: [PATCH 01/15] Mark used and unused APIs by versions. --- .../actions/MarkSegmentsAsUnusedAction.java | 5 +- .../metadata/SegmentsMetadataManager.java | 32 ++- .../metadata/SqlSegmentsMetadataManager.java | 35 ++- .../metadata/SqlSegmentsMetadataQuery.java | 90 +++++-- .../server/http/DataSourcesResource.java | 28 +- .../SqlSegmentsMetadataManagerTest.java | 249 ++++++++++++++++++ .../druid/metadata/TestDerbyConnector.java | 2 +- .../simulate/TestSegmentsMetadataManager.java | 4 +- .../server/http/DataSourcesResourceTest.java | 37 +-- 9 files changed, 415 insertions(+), 67 deletions(-) diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/MarkSegmentsAsUnusedAction.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/MarkSegmentsAsUnusedAction.java index ddf57afbc185..93cb75280fac 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/MarkSegmentsAsUnusedAction.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/MarkSegmentsAsUnusedAction.java @@ -63,9 +63,8 @@ public TypeReference getReturnTypeReference() @Override public Integer perform(Task task, TaskActionToolbox toolbox) { - int numMarked = toolbox.getIndexerMetadataStorageCoordinator() - .markSegmentsAsUnusedWithinInterval(dataSource, interval); - return numMarked; + return toolbox.getIndexerMetadataStorageCoordinator() + .markSegmentsAsUnusedWithinInterval(dataSource, interval); } @Override diff --git a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java index 540eba990f22..91e33b7a231c 100644 --- a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java @@ -53,7 +53,20 @@ public interface SegmentsMetadataManager */ int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource); - int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval); + /** + * Marks non-overshadowed unused segments for the given interval as used. + * @return Number of segments updated + */ + default int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval) + { + return markAsUsedNonOvershadowedSegmentsInInterval(dataSource, interval, null); + } + + /** + * Marks non-overshadowed unused segments for the given interval and optional list of versions as used. + * @return Number of segments updated + */ + int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions); /** * Marks the given segment IDs as "used" only if there are not already overshadowed @@ -81,7 +94,22 @@ public interface SegmentsMetadataManager */ int markAsUnusedAllSegmentsInDataSource(String dataSource); - int markAsUnusedSegmentsInInterval(String dataSource, Interval interval); + /** + * Marks segments as unused that are fully contained in the specified interval. Segments that are already marked as + * unused are not updated. + * @return Number of segments updated + */ + default int markAsUnusedSegmentsInInterval(String dataSource, Interval interval) + { + return markAsUnusedSegmentsInInterval(dataSource, interval, null); + } + + /** + * Marks segments as unused that are fully contained in the specified interval with an optional list of versions. + * Segments that are already marked as unused are not updated. + * @return The number of segments updated + */ + int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions); int markSegmentsAsUnused(Set segmentIds); diff --git a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java index 4a268a2257da..66a60e072c02 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java @@ -654,21 +654,25 @@ public boolean markSegmentAsUsed(final String segmentId) @Override public int markAsUsedAllNonOvershadowedSegmentsInDataSource(final String dataSource) { - return doMarkAsUsedNonOvershadowedSegments(dataSource, null); + return doMarkAsUsedNonOvershadowedSegments(dataSource, null, null); } @Override - public int markAsUsedNonOvershadowedSegmentsInInterval(final String dataSource, final Interval interval) + public int markAsUsedNonOvershadowedSegmentsInInterval( + final String dataSource, + final Interval interval, + @Nullable final List versions + ) { Preconditions.checkNotNull(interval); - return doMarkAsUsedNonOvershadowedSegments(dataSource, interval); + return doMarkAsUsedNonOvershadowedSegments(dataSource, interval, versions); } - /** - * Implementation for both {@link #markAsUsedAllNonOvershadowedSegmentsInDataSource} (if the given interval is null) - * and {@link #markAsUsedNonOvershadowedSegmentsInInterval}. - */ - private int doMarkAsUsedNonOvershadowedSegments(String dataSourceName, @Nullable Interval interval) + private int doMarkAsUsedNonOvershadowedSegments( + final String dataSourceName, + final @Nullable Interval interval, + final @Nullable List versions + ) { final List unusedSegments = new ArrayList<>(); final SegmentTimeline timeline = new SegmentTimeline(); @@ -682,12 +686,12 @@ private int doMarkAsUsedNonOvershadowedSegments(String dataSourceName, @Nullable interval == null ? Intervals.ONLY_ETERNITY : Collections.singletonList(interval); try (final CloseableIterator iterator = - queryTool.retrieveUsedSegments(dataSourceName, intervals)) { + queryTool.retrieveUsedSegments(dataSourceName, intervals, versions)) { timeline.addSegments(iterator); } try (final CloseableIterator iterator = - queryTool.retrieveUnusedSegments(dataSourceName, intervals, null, null, null, null, null)) { + queryTool.retrieveUnusedSegments(dataSourceName, intervals, versions, null, null, null, null)) { while (iterator.hasNext()) { final DataSegment dataSegment = iterator.next(); timeline.addSegments(Iterators.singletonIterator(dataSegment)); @@ -796,7 +800,7 @@ private CloseableIterator retrieveUsedSegmentsOverlappingIntervals( private int markSegmentsAsUsed(final List segmentIds) { if (segmentIds.isEmpty()) { - log.info("No segments found to update!"); + log.info("No segments found to mark as used."); return 0; } @@ -856,13 +860,18 @@ public int markSegmentsAsUnused(Set segmentIds) } @Override - public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interval) + public int markAsUnusedSegmentsInInterval( + final String dataSource, + final Interval interval, + @Nullable final List versions + ) { + Preconditions.checkNotNull(interval); try { return connector.getDBI().withHandle( handle -> SqlSegmentsMetadataQuery.forHandle(handle, connector, dbTables.get(), jsonMapper) - .markSegmentsUnused(dataSourceName, interval) + .markSegmentsUnused(dataSource, interval, versions) ); } catch (Exception e) { 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 29465cd665ba..49f6a7de5734 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java @@ -119,11 +119,24 @@ public CloseableIterator retrieveUsedSegments( final String dataSource, final Collection intervals ) + { + return retrieveUsedSegments(dataSource, intervals, null); + } + + /** + * Similar to {@link #retrieveUsedSegments}, but with an additional {@code versions} argument. When {@code versions} + * is specified, all used segments in the specified {@code intervals} and {@code versions} are retrieved. + */ + public CloseableIterator retrieveUsedSegments( + final String dataSource, + final Collection intervals, + final List versions + ) { return retrieveSegments( dataSource, intervals, - null, + versions, IntervalMode.OVERLAPS, true, null, @@ -314,19 +327,34 @@ public int markSegments(final Collection segmentIds, final boolean us /** * Marks all used segments that are *fully contained by* a particular interval as unused. * - * @return the number of segments actually modified. + * @return Number of segments updated. */ public int markSegmentsUnused(final String dataSource, final Interval interval) + { + return markSegmentsUnused(dataSource, interval, null); + } + + /** + * Marks all used segments that are *fully contained by* a particular interval filtered by an optional list of versions + * as unused. + * + * @return Number of segments updated. + */ + public int markSegmentsUnused(final String dataSource, final Interval interval, @Nullable final List versions) { if (Intervals.isEternity(interval)) { - return handle - .createStatement( - StringUtils.format( - "UPDATE %s SET used=:used, used_status_last_updated = :used_status_last_updated " - + "WHERE dataSource = :dataSource AND used = true", - dbTables.getSegmentsTable() - ) + final StringBuilder sb = new StringBuilder(); + sb.append( + StringUtils.format( + "UPDATE %s SET used=:used, used_status_last_updated = :used_status_last_updated " + + "WHERE dataSource = :dataSource AND used = true", + dbTables.getSegmentsTable() ) + ); + appendConditionForVersions(sb, versions); + + return handle + .createStatement(sb.toString()) .bind("dataSource", dataSource) .bind("used", false) .bind("used_status_last_updated", DateTimes.nowUtc().toString()) @@ -336,15 +364,19 @@ public int markSegmentsUnused(final String dataSource, final Interval interval) // Safe to write a WHERE clause with this interval. Note that it is unsafe if the years are different, because // that means extra characters can sneak in. (Consider a query interval like "2000-01-01/2001-01-01" and a // segment interval like "20001/20002".) - return handle - .createStatement( - StringUtils.format( - "UPDATE %s SET used=:used, used_status_last_updated = :used_status_last_updated " - + "WHERE dataSource = :dataSource AND used = true AND %s", - dbTables.getSegmentsTable(), - IntervalMode.CONTAINS.makeSqlCondition(connector.getQuoteString(), ":start", ":end") - ) + final StringBuilder sb = new StringBuilder(); + sb.append( + StringUtils.format( + "UPDATE %s SET used=:used, used_status_last_updated = :used_status_last_updated " + + "WHERE dataSource = :dataSource AND used = true AND %s", + dbTables.getSegmentsTable(), + IntervalMode.CONTAINS.makeSqlCondition(connector.getQuoteString(), ":start", ":end") ) + ); + appendConditionForVersions(sb, versions); + + return handle + .createStatement(sb.toString()) .bind("dataSource", dataSource) .bind("used", false) .bind("start", interval.getStart().toString()) @@ -358,7 +390,7 @@ public int markSegmentsUnused(final String dataSource, final Interval interval) retrieveSegments( dataSource, Collections.singletonList(interval), - null, + versions, IntervalMode.CONTAINS, true, null, @@ -680,12 +712,7 @@ private Query> buildSegmentsTableQuery( appendConditionForIntervalsAndMatchMode(sb, intervals, matchMode, connector); } - if (!CollectionUtils.isNullOrEmpty(versions)) { - final String versionsStr = versions.stream() - .map(version -> "'" + version + "'") - .collect(Collectors.joining(",")); - sb.append(StringUtils.format(" AND version IN (%s)", versionsStr)); - } + appendConditionForVersions(sb, versions); // Add the used_status_last_updated time filter only for unused segments when maxUsedStatusLastUpdatedTime is non-null. final boolean addMaxUsedLastUpdatedTimeFilter = !used && maxUsedStatusLastUpdatedTime != null; @@ -834,6 +861,21 @@ private static int computeNumChangedSegments(List segmentIds, int[] segm return numChangedSegments; } + private static void appendConditionForVersions( + final StringBuilder sb, + final List versions + ) + { + if (CollectionUtils.isNullOrEmpty(versions)) { + return; + } + + final String versionsCsv = versions.stream() + .map(version -> "'" + version + "'") + .collect(Collectors.joining(",")); + sb.append(StringUtils.format(" AND version IN (%s)", versionsCsv)); + } + enum IntervalMode { CONTAINS { diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index d0458243fcfc..921ac02b4ffb 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -217,10 +217,14 @@ public Response markAsUsedNonOvershadowedSegments( .build(); } else { SegmentUpdateOperation operation = () -> { - final Interval interval = payload.getInterval(); + final List versions = payload.getVersions(); if (interval != null) { - return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval); + if (versions != null) { + return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval, versions); + } else { + return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval); + } } else { final Set segmentIds = payload.getSegmentIds(); if (segmentIds == null || segmentIds.isEmpty()) { @@ -266,9 +270,14 @@ public Response markSegmentsAsUnused( } else { SegmentUpdateOperation operation = () -> { final Interval interval = payload.getInterval(); + final List versions = payload.getVersions(); final int numUpdatedSegments; if (interval != null) { - numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval); + if (versions != null) { + numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval, versions); + } else { + numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval); + } } else { final Set segmentIds = payload.getSegmentIds() @@ -995,15 +1004,18 @@ protected static class MarkDataSourceSegmentsPayload { private final Interval interval; private final Set segmentIds; + private final List versions; @JsonCreator public MarkDataSourceSegmentsPayload( @JsonProperty("interval") Interval interval, - @JsonProperty("segmentIds") Set segmentIds + @JsonProperty("segmentIds") Set segmentIds, + @JsonProperty("versions") List versions ) { this.interval = interval; this.segmentIds = segmentIds; + this.versions = versions; } @JsonProperty @@ -1018,9 +1030,15 @@ public Set getSegmentIds() return segmentIds; } + @JsonProperty + public List getVersions() + { + return versions; + } + public boolean isValid() { - return (interval == null ^ segmentIds == null) && (segmentIds == null || !segmentIds.isEmpty()); + return (interval == null ^ segmentIds == null) && (segmentIds == null || !segmentIds.isEmpty()); // fixme } } } diff --git a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java index b177b40c5876..f00382003aae 100644 --- a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java +++ b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java @@ -41,6 +41,7 @@ import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.NoneShardSpec; import org.hamcrest.MatcherAssert; +import org.joda.time.DateTime; import org.joda.time.Duration; import org.joda.time.Interval; import org.joda.time.Period; @@ -589,6 +590,156 @@ public void testMarkAsUsedNonOvershadowedSegments() throws Exception ); } + @Test + public void testMarkAsUsedNonOvershadowedSegmentsInEternityIntervalWithVersions() throws Exception + { + publishWikiSegments(); + sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); + sqlSegmentsMetadataManager.poll(); + Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically()); + + final DataSegment koalaSegment1 = createSegment( + DS.KOALA, + "2017-10-15T00:00:00.000/2017-10-17T00:00:00.000", + "2017-10-15T20:19:12.565Z" + ); + + final DataSegment koalaSegment2 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + "2017-10-16T20:19:12.565Z" + ); + + // Overshadowed by koalaSegment2 + final DataSegment koalaSegment3 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + "2017-10-15T20:19:12.565Z" + ); + + publishUnusedSegments(koalaSegment1, koalaSegment2, koalaSegment3); + + sqlSegmentsMetadataManager.poll(); + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + Assert.assertEquals( + 2, + sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + DS.KOALA, + Intervals.ETERNITY, + ImmutableList.of("2017-10-15T20:19:12.565Z", "2017-10-16T20:19:12.565Z") + ) + ); + sqlSegmentsMetadataManager.poll(); + + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2, koalaSegment1, koalaSegment2), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + } + + @Test + public void testMarkAsUsedNonOvershadowedSegmentsInFiniteIntervalWithVersions() throws Exception + { + publishWikiSegments(); + sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); + sqlSegmentsMetadataManager.poll(); + Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically()); + + final DataSegment koalaSegment1 = createSegment( + DS.KOALA, + "2017-10-15T00:00:00.000/2017-10-17T00:00:00.000", + "2017-10-15T20:19:12.565Z" + ); + + final DataSegment koalaSegment2 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + "2017-10-16T20:19:12.565Z" + ); + + // Overshadowed by koalaSegment2 + final DataSegment koalaSegment3 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + "2017-10-15T20:19:12.565Z" + ); + + publishUnusedSegments(koalaSegment1, koalaSegment2, koalaSegment3); + + sqlSegmentsMetadataManager.poll(); + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + Assert.assertEquals( + 2, + sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + DS.KOALA, + Intervals.of("2017-10-15/2017-10-18"), + ImmutableList.of("2017-10-15T20:19:12.565Z", "2017-10-16T20:19:12.565Z") + ) + ); + sqlSegmentsMetadataManager.poll(); + + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2, koalaSegment1, koalaSegment2), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + } + + @Test + public void testMarkAsUsedNonOvershadowedSegmentsWithNonExistentVersions() throws Exception + { + publishWikiSegments(); + sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); + sqlSegmentsMetadataManager.poll(); + Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically()); + + final DataSegment koalaSegment1 = createSegment( + DS.KOALA, + "2017-10-15T00:00:00.000/2017-10-17T00:00:00.000", + "2017-10-15T20:19:12.565Z" + ); + + final DataSegment koalaSegment2 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + "2017-10-16T20:19:12.565Z" + ); + + // Overshadowed by koalaSegment2 + final DataSegment koalaSegment3 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + "2017-10-15T20:19:12.565Z" + ); + + publishUnusedSegments(koalaSegment1, koalaSegment2, koalaSegment3); + + sqlSegmentsMetadataManager.poll(); + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + Assert.assertEquals( + 0, + sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + DS.KOALA, + Intervals.ETERNITY, + ImmutableList.of("foo", "bar") + ) + ); + sqlSegmentsMetadataManager.poll(); + + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + } + @Test public void testMarkAsUsedNonOvershadowedSegmentsInvalidDataSource() throws Exception { @@ -796,6 +947,104 @@ public void testMarkAsUnusedSegmentsInInterval() throws IOException ); } + @Test + public void testMarkAsUnusedSegmentsInIntervalAndVersions() throws IOException + { + publishWikiSegments(); + sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); + sqlSegmentsMetadataManager.poll(); + Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically()); + + final DateTime now = DateTimes.nowUtc(); + final String v1 = now.toString(); + final String v2 = now.plus(Duration.standardDays(1)).toString(); + + final DataSegment koalaSegment1 = createSegment( + DS.KOALA, + "2017-10-15T00:00:00.000/2017-10-16T00:00:00.000", + v1 + ); + final DataSegment koalaSegment2 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + v2 + ); + final DataSegment koalaSegment3 = createSegment( + DS.KOALA, + "2017-10-19T00:00:00.000/2017-10-20T00:00:00.000", + v2 + ); + + publisher.publishSegment(koalaSegment1); + publisher.publishSegment(koalaSegment2); + publisher.publishSegment(koalaSegment3); + final Interval theInterval = Intervals.of("2017-10-15/2017-10-18"); + + Assert.assertEquals( + 2, + sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval( + DS.KOALA, + theInterval, + ImmutableList.of(v1, v2) + ) + ); + + sqlSegmentsMetadataManager.poll(); + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2, koalaSegment3), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + } + + @Test + public void testMarkAsUnusedSegmentsInIntervalAndNonExistentVersions() throws IOException + { + publishWikiSegments(); + sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); + sqlSegmentsMetadataManager.poll(); + Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically()); + + final DateTime now = DateTimes.nowUtc(); + final String v1 = now.toString(); + final String v2 = now.plus(Duration.standardDays(1)).toString(); + + final DataSegment koalaSegment1 = createSegment( + DS.KOALA, + "2017-10-15T00:00:00.000/2017-10-16T00:00:00.000", + v1 + ); + final DataSegment koalaSegment2 = createSegment( + DS.KOALA, + "2017-10-17T00:00:00.000/2017-10-18T00:00:00.000", + v2 + ); + final DataSegment koalaSegment3 = createSegment( + DS.KOALA, + "2017-10-19T00:00:00.000/2017-10-20T00:00:00.000", + v2 + ); + + publisher.publishSegment(koalaSegment1); + publisher.publishSegment(koalaSegment2); + publisher.publishSegment(koalaSegment3); + final Interval theInterval = Intervals.of("2017-10-15/2017-10-18"); + + Assert.assertEquals( + 0, + sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval( + DS.KOALA, + theInterval, + ImmutableList.of("foo", "bar", "baz") + ) + ); + + sqlSegmentsMetadataManager.poll(); + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2, koalaSegment1, koalaSegment2, koalaSegment3), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + } + @Test public void testMarkAsUnusedSegmentsInIntervalWithOverlappingInterval() throws IOException { diff --git a/server/src/test/java/org/apache/druid/metadata/TestDerbyConnector.java b/server/src/test/java/org/apache/druid/metadata/TestDerbyConnector.java index 1fa2ef2302c9..fc2330cf1f0e 100644 --- a/server/src/test/java/org/apache/druid/metadata/TestDerbyConnector.java +++ b/server/src/test/java/org/apache/druid/metadata/TestDerbyConnector.java @@ -145,7 +145,7 @@ public SegmentsTable segments() } /** - * A wrapper class for queries on the segments table. + * A wrapper class for updating the segments table. */ public static class SegmentsTable { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java index adf12ae70543..356976aa3a27 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java @@ -87,7 +87,7 @@ public int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource) } @Override - public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval) + public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions) { return 0; } @@ -116,7 +116,7 @@ public int markAsUnusedAllSegmentsInDataSource(String dataSource) } @Override - public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval) + public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions) { return 0; } diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index ace68ad3c21e..a74d93920661 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -742,7 +742,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInterval() DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null) + new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, null) ); Assert.assertEquals(200, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -761,7 +761,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalNoneUpdated() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null) + new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, null) ); Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -780,7 +780,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsSet() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, segmentIds) + new DataSourcesResource.MarkDataSourceSegmentsPayload(null, segmentIds, null) ); Assert.assertEquals(200, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -799,7 +799,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null) + new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, null) ); Assert.assertEquals(500, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -818,7 +818,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), null) + new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), null, null) ); Assert.assertEquals(200, response.getStatus()); Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); @@ -832,7 +832,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadNoArguments() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null) + new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, null) ); Assert.assertEquals(400, response.getStatus()); } @@ -844,7 +844,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadBothArguments() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), ImmutableSet.of()) + new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), ImmutableSet.of(), null) ); Assert.assertEquals(400, response.getStatus()); } @@ -856,7 +856,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadEmptyArray() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, ImmutableSet.of()) + new DataSourcesResource.MarkDataSourceSegmentsPayload(null, ImmutableSet.of(), null) ); Assert.assertEquals(400, response.getStatus()); } @@ -1018,7 +1018,8 @@ public void testMarkSegmentsAsUnused() null, segmentIds.stream() .map(SegmentId::toString) - .collect(Collectors.toSet()) + .collect(Collectors.toSet()), + null ); DataSourcesResource dataSourcesResource = createResource(); @@ -1047,7 +1048,8 @@ public void testMarkSegmentsAsUnusedNoChanges() null, segmentIds.stream() .map(SegmentId::toString) - .collect(Collectors.toSet()) + .collect(Collectors.toSet()), + null ); DataSourcesResource dataSourcesResource = createResource(); @@ -1078,7 +1080,8 @@ public void testMarkSegmentsAsUnusedException() null, segmentIds.stream() .map(SegmentId::toString) - .collect(Collectors.toSet()) + .collect(Collectors.toSet()), + null ); DataSourcesResource dataSourcesResource = @@ -1098,7 +1101,7 @@ public void testMarkAsUnusedSegmentsInInterval() EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null); + new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1118,7 +1121,7 @@ public void testMarkAsUnusedSegmentsInIntervalNoChanges() EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null); + new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1139,7 +1142,7 @@ public void testMarkAsUnusedSegmentsInIntervalException() EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null); + new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); @@ -1157,7 +1160,7 @@ public void testMarkAsUnusedSegmentsInIntervalNoDataSource() EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null); + new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1189,7 +1192,7 @@ public void testMarkSegmentsAsUnusedInvalidPayload() createResource(); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null); + new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, null); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); Assert.assertEquals(400, response.getStatus()); @@ -1203,7 +1206,7 @@ public void testMarkSegmentsAsUnusedInvalidPayloadBothArguments() createResource(); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-01/P1D"), ImmutableSet.of()); + new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-01/P1D"), ImmutableSet.of(), null); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); Assert.assertEquals(400, response.getStatus()); From 620a936ea6ca2dd57775bf9798d7eca99caf612c Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 17:27:47 -0700 Subject: [PATCH 02/15] remove the conditional invocations. --- .../druid/server/http/DataSourcesResource.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index 921ac02b4ffb..168071ed5aa1 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -220,11 +220,7 @@ public Response markAsUsedNonOvershadowedSegments( final Interval interval = payload.getInterval(); final List versions = payload.getVersions(); if (interval != null) { - if (versions != null) { - return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval, versions); - } else { - return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval); - } + return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval, versions); } else { final Set segmentIds = payload.getSegmentIds(); if (segmentIds == null || segmentIds.isEmpty()) { @@ -273,11 +269,7 @@ public Response markSegmentsAsUnused( final List versions = payload.getVersions(); final int numUpdatedSegments; if (interval != null) { - if (versions != null) { - numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval, versions); - } else { - numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval); - } + numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval, versions); } else { final Set segmentIds = payload.getSegmentIds() From 853ba6b80fdae856d323e78959d0f5fc210ca363 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 18:48:22 -0700 Subject: [PATCH 03/15] isValid() and test updates. --- .../druid/server/http/DataSourcesResource.java | 16 +++++++++++++--- .../server/http/DataSourcesResourceTest.java | 16 ++++++++-------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index 168071ed5aa1..4f9f0e2d39c5 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -68,6 +68,7 @@ import org.apache.druid.timeline.TimelineObjectHolder; import org.apache.druid.timeline.VersionedIntervalTimeline; import org.apache.druid.timeline.partition.PartitionChunk; +import org.apache.druid.utils.CollectionUtils; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -205,8 +206,8 @@ public Response markAsUsedAllNonOvershadowedSegments(@PathParam("dataSourceName" @Produces(MediaType.APPLICATION_JSON) @ResourceFilters(DatasourceResourceFilter.class) public Response markAsUsedNonOvershadowedSegments( - @PathParam("dataSourceName") String dataSourceName, - MarkDataSourceSegmentsPayload payload + @PathParam("dataSourceName") final String dataSourceName, + final MarkDataSourceSegmentsPayload payload ) { if (payload == null || !payload.isValid()) { @@ -1030,7 +1031,16 @@ public List getVersions() public boolean isValid() { - return (interval == null ^ segmentIds == null) && (segmentIds == null || !segmentIds.isEmpty()); // fixme + if (interval == null && CollectionUtils.isNullOrEmpty(segmentIds)) { + return false; + } + if (interval != null && segmentIds != null) { + return false; + } + if (!CollectionUtils.isNullOrEmpty(versions) && interval == null) { + return false; + } + return true; } } } diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index a74d93920661..e6adce3f9def 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -735,7 +735,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInterval() { Interval interval = Intervals.of("2010-01-22/P1D"); int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); + segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); EasyMock.expect(numUpdatedSegments).andReturn(3).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -753,7 +753,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalNoneUpdated() { Interval interval = Intervals.of("2010-01-22/P1D"); int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); + segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); EasyMock.expect(numUpdatedSegments).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -791,7 +791,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() { Interval interval = Intervals.of("2010-01-22/P1D"); int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); + segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); EasyMock.expect(numUpdatedSegments).andThrow(new RuntimeException("Error!")).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -810,7 +810,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() { Interval interval = Intervals.of("2010-01-22/P1D"); int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); + segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); EasyMock.expect(numUpdatedSegments).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -1097,7 +1097,7 @@ public void testMarkAsUnusedSegmentsInInterval() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)).andReturn(1).once(); + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)).andReturn(1).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = @@ -1117,7 +1117,7 @@ public void testMarkAsUnusedSegmentsInIntervalNoChanges() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)).andReturn(0).once(); + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = @@ -1136,7 +1136,7 @@ public void testMarkAsUnusedSegmentsInIntervalException() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)) + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)) .andThrow(new RuntimeException("Exception occurred")) .once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -1156,7 +1156,7 @@ public void testMarkAsUnusedSegmentsInIntervalException() public void testMarkAsUnusedSegmentsInIntervalNoDataSource() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)).andReturn(0).once(); + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = From 9c48f88c8ae26f83b971ddcef79b3b18912b48ab Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 19:06:27 -0700 Subject: [PATCH 04/15] isValid() and tests. --- .../server/http/DataSourcesResource.java | 7 +--- .../server/http/DataSourcesResourceTest.java | 37 ++++++++++++++++--- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index 4f9f0e2d39c5..574af41ecf4d 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -1029,7 +1029,7 @@ public List getVersions() return versions; } - public boolean isValid() + private boolean isValid() { if (interval == null && CollectionUtils.isNullOrEmpty(segmentIds)) { return false; @@ -1037,10 +1037,7 @@ public boolean isValid() if (interval != null && segmentIds != null) { return false; } - if (!CollectionUtils.isNullOrEmpty(versions) && interval == null) { - return false; - } - return true; + return CollectionUtils.isNullOrEmpty(versions) || interval != null; } } } diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index e6adce3f9def..5539cfa02fc0 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -826,7 +826,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadNoArguments() + public void testMarkAsUsedNonOvershadowedSegmentsInvalidNoArguments() { DataSourcesResource dataSourcesResource = createResource(); @@ -838,7 +838,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadNoArguments() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadBothArguments() + public void testMarkAsUsedNonOvershadowedSegmentsInvalidBothIntervalAndSegmentIds() { DataSourcesResource dataSourcesResource = createResource(); @@ -850,7 +850,19 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadBothArguments() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadEmptyArray() + public void testMarkAsUsedNonOvershadowedSegmentsInvalidAllParamsSpecified() + { + DataSourcesResource dataSourcesResource = createResource(); + + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2020/2030"), ImmutableSet.of("seg1"), ImmutableList.of("v1", "v2")) + ); + Assert.assertEquals(400, response.getStatus()); + } + + @Test + public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptySegmentIdsArray() { DataSourcesResource dataSourcesResource = createResource(); @@ -862,11 +874,26 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadEmptyArray() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsNoPayload() + public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptyVersionsArray() { DataSourcesResource dataSourcesResource = createResource(); - Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments("datasource1", null); + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, ImmutableList.of()) + ); + Assert.assertEquals(400, response.getStatus()); + } + + @Test + public void testMarkAsUsedNonOvershadowedSegmentsInvalidVersionsNoInterval() + { + DataSourcesResource dataSourcesResource = createResource(); + + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.MarkDataSourceSegmentsPayload(null, ImmutableSet.of(), ImmutableList.of("v1", "v2")) + ); Assert.assertEquals(400, response.getStatus()); } From dbb2846a15c0c224d81cabeb4a26eb9fa37e856c Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 19:09:35 -0700 Subject: [PATCH 05/15] Remove warning logs for invalid user requests. Also, downgrade visibility. --- .../org/apache/druid/server/http/DataSourcesResource.java | 4 +--- .../org/apache/druid/server/http/DataSourcesResourceTest.java | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index 574af41ecf4d..bd758de7f3b4 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -211,7 +211,6 @@ public Response markAsUsedNonOvershadowedSegments( ) { if (payload == null || !payload.isValid()) { - log.warn("Invalid request payload: [%s]", payload); return Response .status(Response.Status.BAD_REQUEST) .entity("Invalid request payload, either interval or segmentIds array must be specified") @@ -259,7 +258,6 @@ public Response markSegmentsAsUnused( ) { if (payload == null || !payload.isValid()) { - log.warn("Invalid request payload: [%s]", payload); return Response .status(Response.Status.BAD_REQUEST) .entity("Invalid request payload, either interval or segmentIds array must be specified") @@ -993,7 +991,7 @@ static boolean isSegmentLoaded(Iterable servedSegments } @VisibleForTesting - protected static class MarkDataSourceSegmentsPayload + static class MarkDataSourceSegmentsPayload { private final Interval interval; private final Set segmentIds; diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index 5539cfa02fc0..d284cb0b4c7e 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -886,13 +886,13 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptyVersionsArray() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidVersionsNoInterval() + public void testMarkAsUsedNonOvershadowedSegmentsInvalidVersionsWithoutInterval() { DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, ImmutableSet.of(), ImmutableList.of("v1", "v2")) + new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, ImmutableList.of("v1", "v2")) ); Assert.assertEquals(400, response.getStatus()); } From 1404ed6dc72103b999028ffcb2d4f19534da1f73 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 19:23:14 -0700 Subject: [PATCH 06/15] Update resp message, etc. --- .../druid/server/http/DataSourcesResource.java | 16 ++++++++++------ .../server/http/DataSourcesResourceTest.java | 3 ++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index bd758de7f3b4..22dccc05d7fe 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -260,7 +260,8 @@ public Response markSegmentsAsUnused( if (payload == null || !payload.isValid()) { return Response .status(Response.Status.BAD_REQUEST) - .entity("Invalid request payload, either interval or segmentIds array must be specified") + .entity("Invalid request payload, either 'interval' or 'segmentIds' array must be specified. 'versions' array " + + "may be optionally specified only when 'interval' is provided.") .build(); } else { SegmentUpdateOperation operation = () -> { @@ -301,7 +302,7 @@ public Response markSegmentsAsUnused( private static Response logAndCreateDataSourceNotFoundResponse(String dataSourceName) { - log.warn("datasource not found [%s]", dataSourceName); + log.warn("datasource[%s] not found", dataSourceName); return Response.noContent().build(); } @@ -318,7 +319,7 @@ private static Response performSegmentUpdate(String dataSourceName, SegmentUpdat .build(); } catch (Exception e) { - log.error(e, "Error occurred while updating segments for data source[%s]", dataSourceName); + log.error(e, "Error occurred while updating segments for datasource[%s]", dataSourceName); return Response .serverError() .entity(ImmutableMap.of("error", "Exception occurred.", "message", Throwables.getRootCause(e).toString())) @@ -999,9 +1000,9 @@ static class MarkDataSourceSegmentsPayload @JsonCreator public MarkDataSourceSegmentsPayload( - @JsonProperty("interval") Interval interval, - @JsonProperty("segmentIds") Set segmentIds, - @JsonProperty("versions") List versions + @JsonProperty("interval") @Nullable Interval interval, + @JsonProperty("segmentIds") @Nullable Set segmentIds, + @JsonProperty("versions") @Nullable List versions ) { this.interval = interval; @@ -1009,18 +1010,21 @@ public MarkDataSourceSegmentsPayload( this.versions = versions; } + @Nullable @JsonProperty public Interval getInterval() { return interval; } + @Nullable @JsonProperty public Set getSegmentIds() { return segmentIds; } + @Nullable @JsonProperty public List getVersions() { diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index d284cb0b4c7e..dc1a8194306e 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -1207,7 +1207,8 @@ public void testMarkSegmentsAsUnusedNullPayload() Assert.assertEquals(400, response.getStatus()); Assert.assertNotNull(response.getEntity()); Assert.assertEquals( - "Invalid request payload, either interval or segmentIds array must be specified", + "Invalid request payload, either 'interval' or 'segmentIds' array must be specified." + + " 'versions' array may be optionally specified only when 'interval' is provided.", response.getEntity() ); } From b93048b7918cfb922e17d9eb4c803e9593f447e4 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 20:28:59 -0700 Subject: [PATCH 07/15] tests and some cleanup. --- .../server/http/DataSourcesResource.java | 18 ++- .../server/http/DataSourcesResourceTest.java | 124 +++++++++++++++--- 2 files changed, 120 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index 22dccc05d7fe..7af805a1eff3 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -118,6 +118,9 @@ public class DataSourcesResource private final DruidCoordinator coordinator; private final AuditManager auditManager; + private final String invalidErrMsg = "Invalid request payload. Specify either 'interval' or 'segmentIds', but not both." + + " Optionally, include 'versions' only when 'interval' is provided."; + @Inject public DataSourcesResource( CoordinatorServerView serverInventoryView, @@ -213,7 +216,7 @@ public Response markAsUsedNonOvershadowedSegments( if (payload == null || !payload.isValid()) { return Response .status(Response.Status.BAD_REQUEST) - .entity("Invalid request payload, either interval or segmentIds array must be specified") + .entity(invalidErrMsg) .build(); } else { SegmentUpdateOperation operation = () -> { @@ -260,8 +263,7 @@ public Response markSegmentsAsUnused( if (payload == null || !payload.isValid()) { return Response .status(Response.Status.BAD_REQUEST) - .entity("Invalid request payload, either 'interval' or 'segmentIds' array must be specified. 'versions' array " - + "may be optionally specified only when 'interval' is provided.") + .entity(invalidErrMsg) .build(); } else { SegmentUpdateOperation operation = () -> { @@ -567,9 +569,9 @@ private SegmentsLoadStatistics computeSegmentLoadStatistics(Iterable servedSegments return false; } + /** + * Either {@code interval} or {@code segmentIds} array must be specified, but not both. + * {@code versions} may be optionally specified only when {@code interval} is provided. + */ @VisibleForTesting static class MarkDataSourceSegmentsPayload { diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index dc1a8194306e..259969ba4890 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -734,8 +734,9 @@ public void testMarkSegmentAsUsedNoChange() public void testMarkAsUsedNonOvershadowedSegmentsInterval() { Interval interval = Intervals.of("2010-01-22/P1D"); - int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); + int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull() + ); EasyMock.expect(numUpdatedSegments).andReturn(3).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -748,12 +749,53 @@ public void testMarkAsUsedNonOvershadowedSegmentsInterval() EasyMock.verify(segmentsMetadataManager, inventoryView, server); } + @Test + public void testMarkAsUsedNonOvershadowedSegmentsIntervalWithVersions() + { + Interval interval = Intervals.of("2010-01-22/P1D"); + + int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.eq(ImmutableList.of("v0")) + ); + EasyMock.expect(numUpdatedSegments).andReturn(3).once(); + EasyMock.replay(segmentsMetadataManager, inventoryView, server); + + DataSourcesResource dataSourcesResource = createResource(); + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, ImmutableList.of("v0")) + ); + Assert.assertEquals(200, response.getStatus()); + EasyMock.verify(segmentsMetadataManager, inventoryView, server); + } + + @Test + public void testMarkAsUsedNonOvershadowedSegmentsIntervalWithNonExistentVersion() + { + Interval interval = Intervals.of("2010-01-22/P1D"); + + int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.eq(ImmutableList.of("foo")) + ); + EasyMock.expect(numUpdatedSegments).andReturn(0).once(); + EasyMock.replay(segmentsMetadataManager, inventoryView, server); + + DataSourcesResource dataSourcesResource = createResource(); + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, ImmutableList.of("foo")) + ); + Assert.assertEquals(200, response.getStatus()); + EasyMock.verify(segmentsMetadataManager, inventoryView, server); + } + @Test public void testMarkAsUsedNonOvershadowedSegmentsIntervalNoneUpdated() { Interval interval = Intervals.of("2010-01-22/P1D"); - int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); + int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull() + ); EasyMock.expect(numUpdatedSegments).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -771,8 +813,9 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalNoneUpdated() public void testMarkAsUsedNonOvershadowedSegmentsSet() { Set segmentIds = ImmutableSet.of(dataSegmentList.get(1).getId().toString()); - int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegments(EasyMock.eq("datasource1"), EasyMock.eq(segmentIds)); + int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegments( + EasyMock.eq("datasource1"), EasyMock.eq(segmentIds) + ); EasyMock.expect(numUpdatedSegments).andReturn(3).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -790,8 +833,9 @@ public void testMarkAsUsedNonOvershadowedSegmentsSet() public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() { Interval interval = Intervals.of("2010-01-22/P1D"); - int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); + int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull() + ); EasyMock.expect(numUpdatedSegments).andThrow(new RuntimeException("Error!")).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -809,8 +853,9 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() { Interval interval = Intervals.of("2010-01-22/P1D"); - int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull()); + int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval( + EasyMock.eq("datasource1"), EasyMock.eq(interval), EasyMock.isNull() + ); EasyMock.expect(numUpdatedSegments).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -844,7 +889,9 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidBothIntervalAndSegmentId Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), ImmutableSet.of(), null) + new DataSourcesResource.MarkDataSourceSegmentsPayload( + Intervals.of("2010-01-22/P1D"), ImmutableSet.of(), null + ) ); Assert.assertEquals(400, response.getStatus()); } @@ -856,7 +903,9 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidAllParamsSpecified() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2020/2030"), ImmutableSet.of("seg1"), ImmutableList.of("v1", "v2")) + new DataSourcesResource.MarkDataSourceSegmentsPayload( + Intervals.of("2020/2030"), ImmutableSet.of("seg1"), ImmutableList.of("v1", "v2") + ) ); Assert.assertEquals(400, response.getStatus()); } @@ -892,7 +941,11 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidVersionsWithoutInterval( Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, ImmutableList.of("v1", "v2")) + new DataSourcesResource.MarkDataSourceSegmentsPayload( + null, + null, + ImmutableList.of("v1", "v2") + ) ); Assert.assertEquals(400, response.getStatus()); } @@ -1183,7 +1236,8 @@ public void testMarkAsUnusedSegmentsInIntervalException() public void testMarkAsUnusedSegmentsInIntervalNoDataSource() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)).andReturn(0).once(); + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)) + .andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); final DataSourcesResource.MarkDataSourceSegmentsPayload payload = @@ -1197,6 +1251,44 @@ public void testMarkAsUnusedSegmentsInIntervalNoDataSource() EasyMock.verify(segmentsMetadataManager); } + @Test + public void testMarkAsUnusedSegmentsInIntervalWithVersions() + { + final Interval theInterval = Intervals.of("2010-01-01/P1D"); + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, ImmutableList.of("v1"))) + .andReturn(2).once(); + EasyMock.replay(segmentsMetadataManager, inventoryView, server); + + final DataSourcesResource.MarkDataSourceSegmentsPayload payload = + new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, ImmutableList.of("v1")); + DataSourcesResource dataSourcesResource = createResource(); + prepareRequestForAudit(); + + Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(ImmutableMap.of("numChangedSegments", 2), response.getEntity()); + EasyMock.verify(segmentsMetadataManager); + } + + @Test + public void testMarkAsUnusedSegmentsInIntervalWithNonExistentVersion() + { + final Interval theInterval = Intervals.of("2010-01-01/P1D"); + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, ImmutableList.of("foo"))) + .andReturn(0).once(); + EasyMock.replay(segmentsMetadataManager, inventoryView, server); + + final DataSourcesResource.MarkDataSourceSegmentsPayload payload = + new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, ImmutableList.of("foo")); + DataSourcesResource dataSourcesResource = createResource(); + prepareRequestForAudit(); + + Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); + EasyMock.verify(segmentsMetadataManager); + } + @Test public void testMarkSegmentsAsUnusedNullPayload() { @@ -1207,8 +1299,8 @@ public void testMarkSegmentsAsUnusedNullPayload() Assert.assertEquals(400, response.getStatus()); Assert.assertNotNull(response.getEntity()); Assert.assertEquals( - "Invalid request payload, either 'interval' or 'segmentIds' array must be specified." - + " 'versions' array may be optionally specified only when 'interval' is provided.", + "Invalid request payload. Specify either 'interval' or 'segmentIds', but not both." + + " Optionally, include 'versions' only when 'interval' is provided.", response.getEntity() ); } From 40a47f719b0ec98852b552eaebf28d33c9356c37 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 20:43:31 -0700 Subject: [PATCH 08/15] Docs draft --- docs/api-reference/data-management-api.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/api-reference/data-management-api.md b/docs/api-reference/data-management-api.md index 4adeaa8b2083..3f9424d36ba6 100644 --- a/docs/api-reference/data-management-api.md +++ b/docs/api-reference/data-management-api.md @@ -206,6 +206,8 @@ Marks the state of a group of segments as unused, using an array of segment IDs Pass the array of segment IDs or interval as a JSON object in the request body. For the interval, specify the start and end times as ISO 8601 strings to identify segments inclusive of the start time and exclusive of the end time. +Optionally, specify an array of segment versions with interval. + Druid only updates the segments completely contained within the specified interval; partially overlapping segments are not affected. #### URL @@ -214,12 +216,13 @@ Druid only updates the segments completely contained within the specified interv #### Request body -The group of segments is sent as a JSON request payload that accepts one of the following properties: +The group of segments is sent as a JSON request payload that accepts the following properties: |Property|Description|Example| |----------|-------------|---------| |`interval`|ISO 8601 segments interval.|`"2015-09-12T03:00:00.000Z/2015-09-12T05:00:00.000Z"`| |`segmentIds`|Array of segment IDs.|`["segmentId1", "segmentId2"]`| +|`versions`|Array of segment versions. Must be provided with `interval`.|`["2024-03-14T16:00:04.086Z", ""2024-03-12T16:00:04.086Z"]`| #### Responses @@ -306,6 +309,8 @@ Marks the state of a group of segments as used, using an array of segment IDs or Pass the array of segment IDs or interval as a JSON object in the request body. For the interval, specify the start and end times as ISO 8601 strings to identify segments inclusive of the start time and exclusive of the end time. +Optionally, specify an array of segment versions with interval. + Druid only updates the segments completely contained within the specified interval; partially overlapping segments are not affected. #### URL @@ -314,12 +319,13 @@ Druid only updates the segments completely contained within the specified interv #### Request body -The group of segments is sent as a JSON request payload that accepts one of the following properties: +The group of segments is sent as a JSON request payload that accepts the following properties: |Property|Description|Example| |----------|-------------|---------| |`interval`| ISO 8601 segments interval.|`"2015-09-12T03:00:00.000Z/2015-09-12T05:00:00.000Z"`| |`segmentIds`|Array of segment IDs.|`["segmentId1", "segmentId2"]`| +|`versions`|Array of segment versions. Must be provided with an `interval`.|`["2024-03-14T16:00:04.086Z", ""2024-03-12T16:00:04.086Z"]`| #### Responses From d9710c9633915999f366bc3ade97968ea5fea37f Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 20:58:08 -0700 Subject: [PATCH 09/15] Clarify docs --- docs/api-reference/data-management-api.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/docs/api-reference/data-management-api.md b/docs/api-reference/data-management-api.md index 3f9424d36ba6..8c8d4d6440ae 100644 --- a/docs/api-reference/data-management-api.md +++ b/docs/api-reference/data-management-api.md @@ -206,9 +206,8 @@ Marks the state of a group of segments as unused, using an array of segment IDs Pass the array of segment IDs or interval as a JSON object in the request body. For the interval, specify the start and end times as ISO 8601 strings to identify segments inclusive of the start time and exclusive of the end time. -Optionally, specify an array of segment versions with interval. - -Druid only updates the segments completely contained within the specified interval; partially overlapping segments are not affected. +Optionally, specify an array of segment versions with interval. Druid updates only the segments completely contained +within the specified interval that match the optional list of versions; partially overlapping segments are not affected. #### URL @@ -309,9 +308,8 @@ Marks the state of a group of segments as used, using an array of segment IDs or Pass the array of segment IDs or interval as a JSON object in the request body. For the interval, specify the start and end times as ISO 8601 strings to identify segments inclusive of the start time and exclusive of the end time. -Optionally, specify an array of segment versions with interval. - -Druid only updates the segments completely contained within the specified interval; partially overlapping segments are not affected. +Optionally, specify an array of segment versions with interval. Druid updates only the segments completely contained +within the specified interval that match the optional list of versions; partially overlapping segments are not affected. #### URL From 17a196898b7aeb4d0bda623f0a79e01d696e746f Mon Sep 17 00:00:00 2001 From: Abhishek Radhakrishnan Date: Sun, 17 Mar 2024 22:32:44 -0700 Subject: [PATCH 10/15] Update server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java Co-authored-by: Kashif Faraz --- .../java/org/apache/druid/server/http/DataSourcesResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index 7af805a1eff3..5a478d254fdb 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -118,7 +118,7 @@ public class DataSourcesResource private final DruidCoordinator coordinator; private final AuditManager auditManager; - private final String invalidErrMsg = "Invalid request payload. Specify either 'interval' or 'segmentIds', but not both." + private static final String INVALID_PAYLOAD_ERROR_MESSAGE = "Invalid request payload. Specify either 'interval' or 'segmentIds', but not both." + " Optionally, include 'versions' only when 'interval' is provided."; @Inject From e4a83f2d4314b73296852e371d73080bd9ad1214 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Sun, 17 Mar 2024 22:52:13 -0700 Subject: [PATCH 11/15] Review comments --- .../metadata/SqlSegmentsMetadataQuery.java | 29 +++--- .../server/http/DataSourcesResource.java | 18 ++-- .../server/http/DataSourcesResourceTest.java | 91 +++++++++++-------- 3 files changed, 81 insertions(+), 57 deletions(-) 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 49f6a7de5734..5bb850d5d41c 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java @@ -147,7 +147,7 @@ public CloseableIterator retrieveUsedSegments( } /** - * Retrieves segments for a given datasource that are marked unused and that are *fully contained by* any interval + * Retrieves segments for a given datasource that are marked unused and that are fully contained by any interval * in a particular collection of intervals. If the collection of intervals is empty, this method will retrieve all * unused segments. *

@@ -197,7 +197,7 @@ public CloseableIterator retrieveUnusedSegments( /** * Similar to {@link #retrieveUnusedSegments}, but also retrieves associated metadata for the segments for a given - * datasource that are marked unused and that are *fully contained by* any interval in a particular collection of + * datasource that are marked unused and that are fully contained by any interval in a particular collection of * intervals. If the collection of intervals is empty, this method will retrieve all unused segments. * * This call does not return any information about realtime segments. @@ -325,7 +325,7 @@ public int markSegments(final Collection segmentIds, final boolean us } /** - * Marks all used segments that are *fully contained by* a particular interval as unused. + * Marks all used segments that are fully contained by a particular interval as unused. * * @return Number of segments updated. */ @@ -335,7 +335,7 @@ public int markSegmentsUnused(final String dataSource, final Interval interval) } /** - * Marks all used segments that are *fully contained by* a particular interval filtered by an optional list of versions + * Marks all used segments that are fully contained by a particular interval filtered by an optional list of versions * as unused. * * @return Number of segments updated. @@ -351,7 +351,10 @@ public int markSegmentsUnused(final String dataSource, final Interval interval, dbTables.getSegmentsTable() ) ); - appendConditionForVersions(sb, versions); + + if (!CollectionUtils.isNullOrEmpty(versions)) { + sb.append(getConditionForVersions(versions)); + } return handle .createStatement(sb.toString()) @@ -373,7 +376,10 @@ public int markSegmentsUnused(final String dataSource, final Interval interval, IntervalMode.CONTAINS.makeSqlCondition(connector.getQuoteString(), ":start", ":end") ) ); - appendConditionForVersions(sb, versions); + + if (!CollectionUtils.isNullOrEmpty(versions)) { + sb.append(getConditionForVersions(versions)); + } return handle .createStatement(sb.toString()) @@ -712,7 +718,9 @@ private Query> buildSegmentsTableQuery( appendConditionForIntervalsAndMatchMode(sb, intervals, matchMode, connector); } - appendConditionForVersions(sb, versions); + if (!CollectionUtils.isNullOrEmpty(versions)) { + sb.append(getConditionForVersions(versions)); + } // Add the used_status_last_updated time filter only for unused segments when maxUsedStatusLastUpdatedTime is non-null. final boolean addMaxUsedLastUpdatedTimeFilter = !used && maxUsedStatusLastUpdatedTime != null; @@ -861,19 +869,18 @@ private static int computeNumChangedSegments(List segmentIds, int[] segm return numChangedSegments; } - private static void appendConditionForVersions( - final StringBuilder sb, + private static String getConditionForVersions( final List versions ) { if (CollectionUtils.isNullOrEmpty(versions)) { - return; + return ""; } final String versionsCsv = versions.stream() .map(version -> "'" + version + "'") .collect(Collectors.joining(",")); - sb.append(StringUtils.format(" AND version IN (%s)", versionsCsv)); + return StringUtils.format(" AND version IN (%s)", versionsCsv); } enum IntervalMode diff --git a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java index 5a478d254fdb..c51f03874941 100644 --- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java +++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java @@ -118,9 +118,6 @@ public class DataSourcesResource private final DruidCoordinator coordinator; private final AuditManager auditManager; - private static final String INVALID_PAYLOAD_ERROR_MESSAGE = "Invalid request payload. Specify either 'interval' or 'segmentIds', but not both." - + " Optionally, include 'versions' only when 'interval' is provided."; - @Inject public DataSourcesResource( CoordinatorServerView serverInventoryView, @@ -210,13 +207,13 @@ public Response markAsUsedAllNonOvershadowedSegments(@PathParam("dataSourceName" @ResourceFilters(DatasourceResourceFilter.class) public Response markAsUsedNonOvershadowedSegments( @PathParam("dataSourceName") final String dataSourceName, - final MarkDataSourceSegmentsPayload payload + final SegmentsToUpdateFilter payload ) { if (payload == null || !payload.isValid()) { return Response .status(Response.Status.BAD_REQUEST) - .entity(invalidErrMsg) + .entity(SegmentsToUpdateFilter.INVALID_PAYLOAD_ERROR_MESSAGE) .build(); } else { SegmentUpdateOperation operation = () -> { @@ -256,14 +253,14 @@ public Response markAsUsedNonOvershadowedSegments( @Consumes(MediaType.APPLICATION_JSON) public Response markSegmentsAsUnused( @PathParam("dataSourceName") final String dataSourceName, - final MarkDataSourceSegmentsPayload payload, + final SegmentsToUpdateFilter payload, @Context final HttpServletRequest req ) { if (payload == null || !payload.isValid()) { return Response .status(Response.Status.BAD_REQUEST) - .entity(invalidErrMsg) + .entity(SegmentsToUpdateFilter.INVALID_PAYLOAD_ERROR_MESSAGE) .build(); } else { SegmentUpdateOperation operation = () -> { @@ -998,14 +995,17 @@ static boolean isSegmentLoaded(Iterable servedSegments * {@code versions} may be optionally specified only when {@code interval} is provided. */ @VisibleForTesting - static class MarkDataSourceSegmentsPayload + static class SegmentsToUpdateFilter { private final Interval interval; private final Set segmentIds; private final List versions; + private static final String INVALID_PAYLOAD_ERROR_MESSAGE = "Invalid request payload. Specify either 'interval' or 'segmentIds', but not both." + + " Optionally, include 'versions' only when 'interval' is provided."; + @JsonCreator - public MarkDataSourceSegmentsPayload( + public SegmentsToUpdateFilter( @JsonProperty("interval") @Nullable Interval interval, @JsonProperty("segmentIds") @Nullable Set segmentIds, @JsonProperty("versions") @Nullable List versions diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index 259969ba4890..923769f47782 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -19,6 +19,8 @@ package org.apache.druid.server.http; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -36,6 +38,7 @@ import org.apache.druid.client.ImmutableSegmentLoadInfo; import org.apache.druid.client.SegmentLoadInfo; import org.apache.druid.error.DruidExceptionMatcher; +import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.metadata.MetadataRuleManager; import org.apache.druid.metadata.SegmentsMetadataManager; @@ -743,7 +746,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInterval() DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, null) + new DataSourcesResource.SegmentsToUpdateFilter(interval, null, null) ); Assert.assertEquals(200, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -763,7 +766,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalWithVersions() DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, ImmutableList.of("v0")) + new DataSourcesResource.SegmentsToUpdateFilter(interval, null, ImmutableList.of("v0")) ); Assert.assertEquals(200, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -783,7 +786,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalWithNonExistentVersion( DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, ImmutableList.of("foo")) + new DataSourcesResource.SegmentsToUpdateFilter(interval, null, ImmutableList.of("foo")) ); Assert.assertEquals(200, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -803,7 +806,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalNoneUpdated() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, null) + new DataSourcesResource.SegmentsToUpdateFilter(interval, null, null) ); Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -823,7 +826,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsSet() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, segmentIds, null) + new DataSourcesResource.SegmentsToUpdateFilter(null, segmentIds, null) ); Assert.assertEquals(200, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -843,7 +846,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, null) + new DataSourcesResource.SegmentsToUpdateFilter(interval, null, null) ); Assert.assertEquals(500, response.getStatus()); EasyMock.verify(segmentsMetadataManager, inventoryView, server); @@ -863,7 +866,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), null, null) + new DataSourcesResource.SegmentsToUpdateFilter(Intervals.of("2010-01-22/P1D"), null, null) ); Assert.assertEquals(200, response.getStatus()); Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); @@ -877,7 +880,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidNoArguments() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, null) + new DataSourcesResource.SegmentsToUpdateFilter(null, null, null) ); Assert.assertEquals(400, response.getStatus()); } @@ -889,7 +892,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidBothIntervalAndSegmentId Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload( + new DataSourcesResource.SegmentsToUpdateFilter( Intervals.of("2010-01-22/P1D"), ImmutableSet.of(), null ) ); @@ -903,7 +906,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidAllParamsSpecified() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload( + new DataSourcesResource.SegmentsToUpdateFilter( Intervals.of("2020/2030"), ImmutableSet.of("seg1"), ImmutableList.of("v1", "v2") ) ); @@ -917,7 +920,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptySegmentIdsArray() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, ImmutableSet.of(), null) + new DataSourcesResource.SegmentsToUpdateFilter(null, ImmutableSet.of(), null) ); Assert.assertEquals(400, response.getStatus()); } @@ -929,7 +932,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptyVersionsArray() Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, ImmutableList.of()) + new DataSourcesResource.SegmentsToUpdateFilter(null, null, ImmutableList.of()) ); Assert.assertEquals(400, response.getStatus()); } @@ -941,7 +944,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidVersionsWithoutInterval( Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload( + new DataSourcesResource.SegmentsToUpdateFilter( null, null, ImmutableList.of("v1", "v2") @@ -1093,8 +1096,8 @@ public void testMarkSegmentsAsUnused() EasyMock.expect(segmentsMetadataManager.markSegmentsAsUnused(segmentIds)).andReturn(1).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload( + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter( null, segmentIds.stream() .map(SegmentId::toString) @@ -1123,8 +1126,8 @@ public void testMarkSegmentsAsUnusedNoChanges() EasyMock.expect(segmentsMetadataManager.markSegmentsAsUnused(segmentIds)).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload( + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter( null, segmentIds.stream() .map(SegmentId::toString) @@ -1155,8 +1158,8 @@ public void testMarkSegmentsAsUnusedException() .once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload( + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter( null, segmentIds.stream() .map(SegmentId::toString) @@ -1180,8 +1183,8 @@ public void testMarkAsUnusedSegmentsInInterval() EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)).andReturn(1).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1200,8 +1203,8 @@ public void testMarkAsUnusedSegmentsInIntervalNoChanges() EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval, null)).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1221,8 +1224,8 @@ public void testMarkAsUnusedSegmentsInIntervalException() .once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); @@ -1240,8 +1243,8 @@ public void testMarkAsUnusedSegmentsInIntervalNoDataSource() .andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, null); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(theInterval, null, null); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1259,8 +1262,8 @@ public void testMarkAsUnusedSegmentsInIntervalWithVersions() .andReturn(2).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, ImmutableList.of("v1")); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(theInterval, null, ImmutableList.of("v1")); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1278,8 +1281,8 @@ public void testMarkAsUnusedSegmentsInIntervalWithNonExistentVersion() .andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null, ImmutableList.of("foo")); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(theInterval, null, ImmutableList.of("foo")); DataSourcesResource dataSourcesResource = createResource(); prepareRequestForAudit(); @@ -1289,11 +1292,25 @@ public void testMarkAsUnusedSegmentsInIntervalWithNonExistentVersion() EasyMock.verify(segmentsMetadataManager); } + @Test + public void testSegmentsToUpdateFilterSerde() throws JsonProcessingException + { + final ObjectMapper mapper = new DefaultObjectMapper(); + final String payload = "{\"interval\":\"2023-01-01T00:00:00.000Z/2024-01-01T00:00:00.000Z\",\"segmentIds\":null,\"versions\":[\"v1\"]}"; + + final DataSourcesResource.SegmentsToUpdateFilter obj = + mapper.readValue(payload, DataSourcesResource.SegmentsToUpdateFilter.class); + Assert.assertEquals(Intervals.of("2023/2024"), obj.getInterval()); + Assert.assertEquals(ImmutableList.of("v1"), obj.getVersions()); + Assert.assertNull(obj.getSegmentIds()); + + Assert.assertEquals(payload, mapper.writeValueAsString(obj)); + } + @Test public void testMarkSegmentsAsUnusedNullPayload() { - DataSourcesResource dataSourcesResource = - createResource(); + DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", null, request); Assert.assertEquals(400, response.getStatus()); @@ -1311,8 +1328,8 @@ public void testMarkSegmentsAsUnusedInvalidPayload() DataSourcesResource dataSourcesResource = createResource(); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null, null); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(null, null, null); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); Assert.assertEquals(400, response.getStatus()); @@ -1325,8 +1342,8 @@ public void testMarkSegmentsAsUnusedInvalidPayloadBothArguments() DataSourcesResource dataSourcesResource = createResource(); - final DataSourcesResource.MarkDataSourceSegmentsPayload payload = - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-01/P1D"), ImmutableSet.of(), null); + final DataSourcesResource.SegmentsToUpdateFilter payload = + new DataSourcesResource.SegmentsToUpdateFilter(Intervals.of("2010-01-01/P1D"), ImmutableSet.of(), null); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); Assert.assertEquals(400, response.getStatus()); From 9302af39ff6b60298d5321198be621508ced68b8 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Sun, 17 Mar 2024 22:55:46 -0700 Subject: [PATCH 12/15] Remove default interface methods only used in tests and update docs. --- docs/api-reference/data-management-api.md | 20 +++++++++--------- .../task/KillUnusedSegmentsTaskTest.java | 21 ++++++++++++------- .../metadata/SegmentsMetadataManager.java | 19 ----------------- .../SqlSegmentsMetadataManagerTest.java | 8 +++---- 4 files changed, 28 insertions(+), 40 deletions(-) diff --git a/docs/api-reference/data-management-api.md b/docs/api-reference/data-management-api.md index 8c8d4d6440ae..754bf62f725a 100644 --- a/docs/api-reference/data-management-api.md +++ b/docs/api-reference/data-management-api.md @@ -217,11 +217,11 @@ within the specified interval that match the optional list of versions; partiall The group of segments is sent as a JSON request payload that accepts the following properties: -|Property|Description|Example| -|----------|-------------|---------| -|`interval`|ISO 8601 segments interval.|`"2015-09-12T03:00:00.000Z/2015-09-12T05:00:00.000Z"`| -|`segmentIds`|Array of segment IDs.|`["segmentId1", "segmentId2"]`| -|`versions`|Array of segment versions. Must be provided with `interval`.|`["2024-03-14T16:00:04.086Z", ""2024-03-12T16:00:04.086Z"]`| +|Property|Description|Required|Example| +|----------|-------------|---------|---------| +|`interval`|ISO 8601 segments interval.|Yes, if `segmentIds` is not specified.|`"2015-09-12T03:00:00.000Z/2015-09-12T05:00:00.000Z"`| +|`segmentIds`|List of segment IDs.|Yes, if `interval` is not specified.|`["segmentId1", "segmentId2"]`| +|`versions`|List of segment versions. Must be provided with `interval`.|No.|`["2024-03-14T16:00:04.086Z", ""2024-03-12T16:00:04.086Z"]`| #### Responses @@ -319,11 +319,11 @@ within the specified interval that match the optional list of versions; partiall The group of segments is sent as a JSON request payload that accepts the following properties: -|Property|Description|Example| -|----------|-------------|---------| -|`interval`| ISO 8601 segments interval.|`"2015-09-12T03:00:00.000Z/2015-09-12T05:00:00.000Z"`| -|`segmentIds`|Array of segment IDs.|`["segmentId1", "segmentId2"]`| -|`versions`|Array of segment versions. Must be provided with an `interval`.|`["2024-03-14T16:00:04.086Z", ""2024-03-12T16:00:04.086Z"]`| +|Property|Description|Required|Example| +|----------|-------------|---------|---------| +|`interval`|ISO 8601 segments interval.|Yes, if `segmentIds` is not specified.|`"2015-09-12T03:00:00.000Z/2015-09-12T05:00:00.000Z"`| +|`segmentIds`|List of segment IDs.|Yes, if `interval` is not specified.|`["segmentId1", "segmentId2"]`| +|`versions`|List of segment versions. Must be provided with `interval`.|No.|`["2024-03-14T16:00:04.086Z", ""2024-03-12T16:00:04.086Z"]`| #### Responses diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java index 1e36cc825a01..382673bed4b8 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTaskTest.java @@ -387,7 +387,8 @@ public void testKillBatchSizeOneAndLimit4() throws Exception segments.size(), getSegmentsMetadataManager().markAsUnusedSegmentsInInterval( DATA_SOURCE, - Intervals.of("2018-01-01/2020-01-01") + Intervals.of("2018-01-01/2020-01-01"), + null ) ); @@ -434,7 +435,8 @@ public void testKillMultipleUnusedSegmentsWithNullMaxUsedStatusLastUpdatedTime() 1, getSegmentsMetadataManager().markAsUnusedSegmentsInInterval( DATA_SOURCE, - segment1.getInterval() + segment1.getInterval(), + null ) ); @@ -442,7 +444,8 @@ public void testKillMultipleUnusedSegmentsWithNullMaxUsedStatusLastUpdatedTime() 1, getSegmentsMetadataManager().markAsUnusedSegmentsInInterval( DATA_SOURCE, - segment4.getInterval() + segment4.getInterval(), + null ) ); @@ -450,7 +453,8 @@ public void testKillMultipleUnusedSegmentsWithNullMaxUsedStatusLastUpdatedTime() 1, getSegmentsMetadataManager().markAsUnusedSegmentsInInterval( DATA_SOURCE, - segment3.getInterval() + segment3.getInterval(), + null ) ); @@ -508,7 +512,8 @@ public void testKillMultipleUnusedSegmentsWithDifferentMaxUsedStatusLastUpdatedT 1, getSegmentsMetadataManager().markAsUnusedSegmentsInInterval( DATA_SOURCE, - segment1.getInterval() + segment1.getInterval(), + null ) ); @@ -516,7 +521,8 @@ public void testKillMultipleUnusedSegmentsWithDifferentMaxUsedStatusLastUpdatedT 1, getSegmentsMetadataManager().markAsUnusedSegmentsInInterval( DATA_SOURCE, - segment4.getInterval() + segment4.getInterval(), + null ) ); @@ -529,7 +535,8 @@ public void testKillMultipleUnusedSegmentsWithDifferentMaxUsedStatusLastUpdatedT 1, getSegmentsMetadataManager().markAsUnusedSegmentsInInterval( DATA_SOURCE, - segment3.getInterval() + segment3.getInterval(), + null ) ); diff --git a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java index 91e33b7a231c..3c2d40ee3403 100644 --- a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java @@ -53,15 +53,6 @@ public interface SegmentsMetadataManager */ int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource); - /** - * Marks non-overshadowed unused segments for the given interval as used. - * @return Number of segments updated - */ - default int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval) - { - return markAsUsedNonOvershadowedSegmentsInInterval(dataSource, interval, null); - } - /** * Marks non-overshadowed unused segments for the given interval and optional list of versions as used. * @return Number of segments updated @@ -94,16 +85,6 @@ default int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Inter */ int markAsUnusedAllSegmentsInDataSource(String dataSource); - /** - * Marks segments as unused that are fully contained in the specified interval. Segments that are already marked as - * unused are not updated. - * @return Number of segments updated - */ - default int markAsUnusedSegmentsInInterval(String dataSource, Interval interval) - { - return markAsUnusedSegmentsInInterval(dataSource, interval, null); - } - /** * Marks segments as unused that are fully contained in the specified interval with an optional list of versions. * Segments that are already marked as unused are not updated. diff --git a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java index f00382003aae..a7128ce27143 100644 --- a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java +++ b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java @@ -834,7 +834,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInInterval() throws IOException ); // 2 out of 3 segments match the interval - Assert.assertEquals(2, sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(DS.KOALA, theInterval)); + Assert.assertEquals(2, sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(DS.KOALA, theInterval, null)); sqlSegmentsMetadataManager.poll(); Assert.assertEquals( @@ -882,7 +882,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInIntervalWithOverlappingInterv ); // 1 out of 3 segments match the interval, other 2 overlap, only the segment fully contained will be marked unused - Assert.assertEquals(1, sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(DS.KOALA, theInterval)); + Assert.assertEquals(1, sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(DS.KOALA, theInterval, null)); sqlSegmentsMetadataManager.poll(); Assert.assertEquals( @@ -938,7 +938,7 @@ public void testMarkAsUnusedSegmentsInInterval() throws IOException final Interval theInterval = Intervals.of("2017-10-15T00:00:00.000/2017-10-18T00:00:00.000"); // 2 out of 3 segments match the interval - Assert.assertEquals(2, sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(DS.KOALA, theInterval)); + Assert.assertEquals(2, sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(DS.KOALA, theInterval, null)); sqlSegmentsMetadataManager.poll(); Assert.assertEquals( @@ -1071,7 +1071,7 @@ public void testMarkAsUnusedSegmentsInIntervalWithOverlappingInterval() throws I final Interval theInterval = Intervals.of("2017-10-16T00:00:00.000/2017-10-20T00:00:00.000"); // 1 out of 3 segments match the interval, other 2 overlap, only the segment fully contained will be marked unused - Assert.assertEquals(1, sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(DS.KOALA, theInterval)); + Assert.assertEquals(1, sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(DS.KOALA, theInterval, null)); sqlSegmentsMetadataManager.poll(); Assert.assertEquals( From 8cc3a1b0128f9f9ef3089f5e7f6a24e3728cf60e Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Mon, 18 Mar 2024 01:35:15 -0700 Subject: [PATCH 13/15] Clarify javadocs and @Nullable. --- .../druid/metadata/SegmentsMetadataManager.java | 11 +++++++---- .../simulate/TestSegmentsMetadataManager.java | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java index 3c2d40ee3403..f0b9f06425d9 100644 --- a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java @@ -54,10 +54,12 @@ public interface SegmentsMetadataManager int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource); /** - * Marks non-overshadowed unused segments for the given interval and optional list of versions as used. + * Marks non-overshadowed unused segments for the given interval and optional list of versions + * as used. If versions are not specified, all versions of non-overshadowed unused segments in the interval + * will be marked as used. * @return Number of segments updated */ - int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions); + int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, @Nullable List versions); /** * Marks the given segment IDs as "used" only if there are not already overshadowed @@ -86,11 +88,12 @@ public interface SegmentsMetadataManager int markAsUnusedAllSegmentsInDataSource(String dataSource); /** - * Marks segments as unused that are fully contained in the specified interval with an optional list of versions. + * Marks segments as unused that are fully contained in the given interval for an optional list of versions. + * If versions are not specified, all versions of segments in the interval will be marked as unused. * Segments that are already marked as unused are not updated. * @return The number of segments updated */ - int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions); + int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, @Nullable List versions); int markSegmentsAsUnused(Set segmentIds); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java index 356976aa3a27..d255d0abc7d4 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java @@ -87,7 +87,7 @@ public int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource) } @Override - public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions) + public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, @Nullable List versions) { return 0; } @@ -116,7 +116,7 @@ public int markAsUnusedAllSegmentsInDataSource(String dataSource) } @Override - public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions) + public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, @Nullable List versions) { return 0; } From 33ed1ced90eaa5929d02da9c68d6a4a5b298f670 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Mon, 18 Mar 2024 01:35:19 -0700 Subject: [PATCH 14/15] Add more tests. --- .../server/http/DataSourcesResourceTest.java | 64 ++++++++++++++----- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index 8b60bc2c9ab1..f6bccbeb7da4 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -874,7 +874,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsWithNullIntervalAndSegmentIds() + public void testMarkAsUsedNonOvershadowedSegmentsWithNullIntervalAndSegmentIdsAndVersions() { DataSourcesResource dataSourcesResource = createResource(); @@ -901,7 +901,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndEmptySegm } @Test - public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndNullSegmentIds() + public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullInterval() { DataSourcesResource dataSourcesResource = createResource(); @@ -926,7 +926,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndSegmentId } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidAllParamsSpecified() + public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndSegmentIdsAndVersions() { DataSourcesResource dataSourcesResource = createResource(); @@ -940,7 +940,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidAllParamsSpecified() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptySegmentIdsArray() + public void testMarkAsUsedNonOvershadowedSegmentsWithEmptySegmentIds() { DataSourcesResource dataSourcesResource = createResource(); @@ -952,7 +952,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptySegmentIdsArray() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptyVersionsArray() + public void testMarkAsUsedNonOvershadowedSegmentsWithEmptyVersions() { DataSourcesResource dataSourcesResource = createResource(); @@ -964,21 +964,55 @@ public void testMarkAsUsedNonOvershadowedSegmentsInvalidEmptyVersionsArray() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsInvalidVersionsWithoutInterval() + public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullVersions() { DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.SegmentsToUpdateFilter( - null, - null, - ImmutableList.of("v1", "v2") - ) + new DataSourcesResource.SegmentsToUpdateFilter(null, null, ImmutableList.of("v1", "v2")) + ); + Assert.assertEquals(400, response.getStatus()); + } + + @Test + public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullSegmentIdsAndVersions() + { + DataSourcesResource dataSourcesResource = createResource(); + + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.SegmentsToUpdateFilter(null, ImmutableSet.of("segment1"), ImmutableList.of("v1", "v2")) ); Assert.assertEquals(400, response.getStatus()); } + @Test + public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndVersions() + { + DataSourcesResource dataSourcesResource = createResource(); + + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.SegmentsToUpdateFilter(Intervals.ETERNITY, null, ImmutableList.of("v1", "v2")) + ); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); + } + + @Test + public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndEmptyVersions() + { + DataSourcesResource dataSourcesResource = createResource(); + + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.SegmentsToUpdateFilter(Intervals.ETERNITY, null, ImmutableList.of()) + ); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); + } + @Test public void testSegmentLoadChecksForVersion() { @@ -1193,8 +1227,7 @@ public void testMarkSegmentsAsUnusedException() null ); - DataSourcesResource dataSourcesResource = - createResource(); + DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); Assert.assertEquals(500, response.getStatus()); Assert.assertNotNull(response.getEntity()); @@ -1253,8 +1286,7 @@ public void testMarkAsUnusedSegmentsInIntervalException() final DataSourcesResource.SegmentsToUpdateFilter payload = new DataSourcesResource.SegmentsToUpdateFilter(theInterval, null, null); - DataSourcesResource dataSourcesResource = - createResource(); + DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); Assert.assertEquals(500, response.getStatus()); Assert.assertNotNull(response.getEntity()); @@ -1349,7 +1381,7 @@ public void testMarkSegmentsAsUnusedNullPayload() } @Test - public void testMarkSegmentsAsUnusedWithNullIntervalAndSegmentIds() + public void testMarkSegmentsAsUnusedWithNullIntervalAndSegmentIdsAndVersions() { DataSourcesResource dataSourcesResource = createResource(); From a1c1f966278146bde64ac28df037d0e87788cd2d Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Mon, 18 Mar 2024 12:29:59 -0700 Subject: [PATCH 15/15] Parameterized versions. --- .../metadata/SqlSegmentsMetadataQuery.java | 69 ++++++++++++++----- 1 file changed, 53 insertions(+), 16 deletions(-) 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 5bb850d5d41c..5308f9dd7fea 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java @@ -42,6 +42,8 @@ import org.skife.jdbi.v2.PreparedBatch; import org.skife.jdbi.v2.Query; import org.skife.jdbi.v2.ResultIterator; +import org.skife.jdbi.v2.SQLStatement; +import org.skife.jdbi.v2.Update; import javax.annotation.Nullable; import java.util.ArrayList; @@ -352,16 +354,23 @@ public int markSegmentsUnused(final String dataSource, final Interval interval, ) ); - if (!CollectionUtils.isNullOrEmpty(versions)) { + final boolean hasVersions = !CollectionUtils.isNullOrEmpty(versions); + + if (hasVersions) { sb.append(getConditionForVersions(versions)); } - return handle + final Update stmt = handle .createStatement(sb.toString()) .bind("dataSource", dataSource) .bind("used", false) - .bind("used_status_last_updated", DateTimes.nowUtc().toString()) - .execute(); + .bind("used_status_last_updated", DateTimes.nowUtc().toString()); + + if (hasVersions) { + bindVersionsToQuery(stmt, versions); + } + + return stmt.execute(); } else if (Intervals.canCompareEndpointsAsStrings(interval) && interval.getStart().getYear() == interval.getEnd().getYear()) { // Safe to write a WHERE clause with this interval. Note that it is unsafe if the years are different, because @@ -377,18 +386,24 @@ public int markSegmentsUnused(final String dataSource, final Interval interval, ) ); - if (!CollectionUtils.isNullOrEmpty(versions)) { + final boolean hasVersions = !CollectionUtils.isNullOrEmpty(versions); + + if (hasVersions) { sb.append(getConditionForVersions(versions)); } - return handle + final Update stmt = handle .createStatement(sb.toString()) .bind("dataSource", dataSource) .bind("used", false) .bind("start", interval.getStart().toString()) .bind("end", interval.getEnd().toString()) - .bind("used_status_last_updated", DateTimes.nowUtc().toString()) - .execute(); + .bind("used_status_last_updated", DateTimes.nowUtc().toString()); + + if (hasVersions) { + bindVersionsToQuery(stmt, versions); + } + return stmt.execute(); } else { // Retrieve, then drop, since we can't write a WHERE clause directly. final List segments = ImmutableList.copyOf( @@ -718,7 +733,9 @@ private Query> buildSegmentsTableQuery( appendConditionForIntervalsAndMatchMode(sb, intervals, matchMode, connector); } - if (!CollectionUtils.isNullOrEmpty(versions)) { + final boolean hasVersions = !CollectionUtils.isNullOrEmpty(versions); + + if (hasVersions) { sb.append(getConditionForVersions(versions)); } @@ -769,6 +786,10 @@ private Query> buildSegmentsTableQuery( bindQueryIntervals(sql, intervals); } + if (hasVersions) { + bindVersionsToQuery(sql, versions); + } + return sql; } @@ -869,18 +890,34 @@ private static int computeNumChangedSegments(List segmentIds, int[] segm return numChangedSegments; } - private static String getConditionForVersions( - final List versions - ) + private static String getConditionForVersions(final List versions) { if (CollectionUtils.isNullOrEmpty(versions)) { return ""; } - final String versionsCsv = versions.stream() - .map(version -> "'" + version + "'") - .collect(Collectors.joining(",")); - return StringUtils.format(" AND version IN (%s)", versionsCsv); + final StringBuilder sb = new StringBuilder(); + + sb.append(" AND version IN ("); + for (int i = 0; i < versions.size(); i++) { + sb.append(StringUtils.format(":version%d", i)); + if (i != versions.size() - 1) { + sb.append(","); + } + } + sb.append(")"); + return sb.toString(); + } + + private static void bindVersionsToQuery(final SQLStatement query, final List versions) + { + if (CollectionUtils.isNullOrEmpty(versions)) { + return; + } + + for (int i = 0; i < versions.size(); i++) { + query.bind(StringUtils.format("version%d", i), versions.get(i)); + } } enum IntervalMode