From 8189b54115cb4657fda77ac7fda762c93de30753 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Thu, 14 Mar 2024 19:46:26 +0530 Subject: [PATCH 01/16] Initial support for mark used and unused APIs by versions. --- .../actions/MarkSegmentsAsUnusedAction.java | 5 +- .../metadata/SegmentsMetadataManager.java | 11 +++ .../metadata/SqlSegmentsMetadataManager.java | 40 +++++++-- .../metadata/SqlSegmentsMetadataQuery.java | 90 ++++++++++++++----- .../server/http/DataSourcesResource.java | 32 +++++-- .../SqlSegmentsMetadataManagerTest.java | 90 +++++++++++++++++++ .../simulate/TestSegmentsMetadataManager.java | 12 +++ .../server/http/DataSourcesResourceTest.java | 35 ++++---- 8 files changed, 262 insertions(+), 53 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..334cba791695 100644 --- a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java @@ -55,6 +55,8 @@ public interface SegmentsMetadataManager int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval); + int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions); + /** * Marks the given segment IDs as "used" only if there are not already overshadowed * by other used segments. Qualifying segment IDs that are already marked as @@ -83,6 +85,15 @@ public interface SegmentsMetadataManager int markAsUnusedSegmentsInInterval(String dataSource, Interval interval); + /** + * TODO fill in javadocs. + * @param dataSource + * @param interval + * @param versions List of versions + * @return + */ + 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..4e90edcddb01 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,33 @@ 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) { Preconditions.checkNotNull(interval); - return doMarkAsUsedNonOvershadowedSegments(dataSource, interval); + return doMarkAsUsedNonOvershadowedSegments(dataSource, interval, null); + } + + @Override + public int markAsUsedNonOvershadowedSegmentsInInterval(final String dataSource, final Interval interval, final List versions) + { + Preconditions.checkNotNull(interval); + Preconditions.checkNotNull(versions); + 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 +694,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)); @@ -700,6 +712,7 @@ private int doMarkAsUsedNonOvershadowedSegments(String dataSourceName, @Nullable } ); + log.info("Unused segments[%s] to be marked as used", unusedSegments); return markNonOvershadowedSegmentsAsUsed(unusedSegments, timeline); } @@ -796,7 +809,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; } @@ -870,6 +883,21 @@ public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interv } } + @Override + public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interval, List versions) + { + try { + return connector.getDBI().withHandle( + handle -> + SqlSegmentsMetadataQuery.forHandle(handle, connector, dbTables.get(), jsonMapper) + .markSegmentsUnused(dataSourceName, interval, versions) + ); + } + catch (Exception e) { + throw new RuntimeException(e); + } + } + @Override public @Nullable ImmutableDruidDataSource getImmutableDataSourceWithUsedSegments(String dataSourceName) { 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..384763441432 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,27 @@ public CloseableIterator retrieveUsedSegments( final String dataSource, final Collection intervals ) + { + return retrieveUsedSegments(dataSource, intervals, null); + } + + /** + * TODO: javadocs + * @param dataSource + * @param intervals + * @param versions + * @return + */ + public CloseableIterator retrieveUsedSegments( + final String dataSource, + final Collection intervals, + final List versions + ) { return retrieveSegments( dataSource, intervals, - null, + versions, IntervalMode.OVERLAPS, true, null, @@ -317,16 +333,30 @@ public int markSegments(final Collection segmentIds, final boolean us * @return the number of segments actually modified. */ public int markSegmentsUnused(final String dataSource, final Interval interval) + { + return markSegmentsUnused(dataSource, interval, null); + } + + /** + * TODO: update javadocs + * Marks all used segments that are *fully contained by* a particular interval as unused. + * + * @return the number of segments actually modified. + */ + 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 +366,20 @@ 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); + log.info("Query[%s]", sb.toString()); + return handle + .createStatement(sb.toString()) .bind("dataSource", dataSource) .bind("used", false) .bind("start", interval.getStart().toString()) @@ -358,7 +393,7 @@ public int markSegmentsUnused(final String dataSource, final Interval interval) retrieveSegments( dataSource, Collections.singletonList(interval), - null, + versions, IntervalMode.CONTAINS, true, null, @@ -474,6 +509,20 @@ public static void appendConditionForIntervalsAndMatchMode( sb.append(")"); } + 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)); + } + /** * Given a Query object bind the input intervals to it * @param query Query to fetch segments @@ -680,12 +729,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; 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 b83bda21dcd0..57e636b9ceb9 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; @@ -212,7 +213,11 @@ public Response markAsUsedNonOvershadowedSegments( SegmentUpdateOperation operation = () -> { final Interval interval = payload.getInterval(); if (interval != null) { - return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval); + if (payload.getVersions() != null) { + return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval, payload.getVersions()); + } else { + return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval); + } } else { final Set segmentIds = payload.getSegmentIds(); if (segmentIds == null || segmentIds.isEmpty()) { @@ -250,8 +255,15 @@ public Response markSegmentsAsUnused( SegmentUpdateOperation operation = () -> { final Interval interval = payload.getInterval(); final int numUpdatedSegments; + // versions by itself or in the presence of an interval + // or make interval optional? if (interval != null) { - numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval); + if (!CollectionUtils.isNullOrEmpty(payload.getVersions())) { + // maybe we should just consolidate into a single function; only tests use this variation. + numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval, payload.getVersions()); + } else { + numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval); + } } else { final Set segmentIds = payload.getSegmentIds() @@ -297,7 +309,8 @@ private Response performSegmentUpdate( final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); if (dataSource == null) { - return logAndCreateDataSourceNotFoundResponse(dataSourceName); + log.info("Datasource[%s] not found, but perhaps there are no used segments.", dataSourceName); +// return logAndCreateDataSourceNotFoundResponse(dataSourceName); } return performSegmentUpdate(dataSourceName, operation); @@ -999,15 +1012,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 @@ -1022,9 +1038,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 7e491325110b..586268423ca7 100644 --- a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java +++ b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java @@ -40,6 +40,7 @@ import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.NoneShardSpec; +import org.assertj.core.api.Assertions; import org.hamcrest.MatcherAssert; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -50,8 +51,10 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.Assertion; import java.io.IOException; +import java.util.Arrays; import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; @@ -585,6 +588,59 @@ public void testMarkAsUsedNonOvershadowedSegments() throws Exception ); } + @Test + public void testMarkAsUsedNonOvershadowedSegments2() 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); + final Set segmentIds = ImmutableSet.of( + koalaSegment1.getId().toString(), + koalaSegment2.getId().toString(), + koalaSegment3.getId().toString() + ); + + 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.markAsUsedNonOvershadowedSegmentsInInterval(DS.KOALA, Intervals.ETERNITY, ImmutableList.of("foo", "bar")) + ); +// Assert.assertEquals(2, sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegments(DS.KOALA, segmentIds)); + sqlSegmentsMetadataManager.poll(); + + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2, koalaSegment1, koalaSegment2), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + } + @Test public void testMarkAsUsedNonOvershadowedSegmentsInvalidDataSource() throws Exception { @@ -792,6 +848,40 @@ public void testMarkAsUnusedSegmentsInInterval() throws IOException ); } + @Test + public void testMarkAsUnusedSegmentsInIntervalAndVersions() throws IOException + { + publishWikiSegments(); + sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); + sqlSegmentsMetadataManager.poll(); + Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically()); + + final DataSegment koalaSegment1 = createNewSegment1(DS.KOALA); + final DataSegment koalaSegment2 = createNewSegment2(DS.KOALA); + final DataSegment koalaSegment3 = createSegment( + DS.KOALA, + "2017-10-19T00:00:00.000/2017-10-20T00:00:00.000", + "2017-10-15T20:19:12.565Z" + ); + + publisher.publishSegment(koalaSegment1); + publisher.publishSegment(koalaSegment2); + publisher.publishSegment(koalaSegment3); + 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, ImmutableList.of("foo", "bar", "2017-10-15T20:19:12.565Z")) + ); + + sqlSegmentsMetadataManager.poll(); + Assert.assertEquals( + ImmutableSet.of(wikiSegment1, wikiSegment2, koalaSegment3), + ImmutableSet.copyOf(sqlSegmentsMetadataManager.iterateAllUsedSegments()) + ); + } + @Test public void testMarkAsUnusedSegmentsInIntervalWithOverlappingInterval() throws IOException { 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..53f51b3d14a0 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 @@ -92,6 +92,12 @@ public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interv return 0; } + @Override + public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions) + { + return 0; + } + @Override public int markAsUsedNonOvershadowedSegments(String dataSource, Set segmentIds) { @@ -121,6 +127,12 @@ public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval) return 0; } + @Override + public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions) + { + return 0; + } + @Override public int markSegmentsAsUnused(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 69fe23a75b5d..6005ef8ea7ac 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 @@ -745,7 +745,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); @@ -767,7 +767,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); @@ -789,7 +789,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); @@ -811,7 +811,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); @@ -828,7 +828,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(204, response.getStatus()); EasyMock.verify(segmentsMetadataManager); @@ -841,7 +841,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()); } @@ -853,7 +853,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()); } @@ -865,7 +865,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()); } @@ -1029,7 +1029,8 @@ public void testMarkSegmentsAsUnused() null, segmentIds.stream() .map(SegmentId::toString) - .collect(Collectors.toSet()) + .collect(Collectors.toSet()), + null ); DataSourcesResource dataSourcesResource = createResource(); @@ -1060,7 +1061,8 @@ public void testMarkSegmentsAsUnusedNoChanges() null, segmentIds.stream() .map(SegmentId::toString) - .collect(Collectors.toSet()) + .collect(Collectors.toSet()), + null ); DataSourcesResource dataSourcesResource = createResource(); @@ -1093,7 +1095,8 @@ public void testMarkSegmentsAsUnusedException() null, segmentIds.stream() .map(SegmentId::toString) - .collect(Collectors.toSet()) + .collect(Collectors.toSet()), + null ); DataSourcesResource dataSourcesResource = @@ -1116,7 +1119,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(); @@ -1139,7 +1142,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(); @@ -1163,7 +1166,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(); @@ -1195,7 +1198,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()); @@ -1209,7 +1212,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 dc869aaaca1be6eff91ccbf5f579d86df61c0d3c Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Thu, 14 Mar 2024 20:00:54 +0530 Subject: [PATCH 02/16] Short circuit only markUnused operation when there are no used segments. We still want to mark used when there are no used segments as we may want to mark a portion of the unused segments as used. --- .../apache/druid/server/http/DataSourcesResource.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 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 b83bda21dcd0..1cf76a663d3f 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 @@ -233,7 +233,7 @@ public Response markAsUsedNonOvershadowedSegments( return segmentsMetadataManager.markAsUsedNonOvershadowedSegments(dataSourceName, segmentIds); } }; - return performSegmentUpdate(dataSourceName, payload, operation); + return performSegmentUpdate(dataSourceName, payload, operation, true); } @POST @@ -278,13 +278,14 @@ public Response markSegmentsAsUnused( ); return numUpdatedSegments; }; - return performSegmentUpdate(dataSourceName, payload, operation); + return performSegmentUpdate(dataSourceName, payload, operation, false); } private Response performSegmentUpdate( String dataSourceName, MarkDataSourceSegmentsPayload payload, - SegmentUpdateOperation operation + SegmentUpdateOperation operation, + boolean isMarkUsed ) { if (payload == null || !payload.isValid()) { @@ -296,7 +297,9 @@ private Response performSegmentUpdate( } final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); - if (dataSource == null) { + // We can skip the markUnused operation when there are no used segments for this datasource. + // However, for markUsed operation, we should still continue as we want to mark a portion of the unused segments as used. + if (dataSource == null && !isMarkUsed) { return logAndCreateDataSourceNotFoundResponse(dataSourceName); } From 86ce0123cbb5de7b0fc09f9d820f1ab8247ebaa9 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Thu, 14 Mar 2024 21:24:52 +0530 Subject: [PATCH 03/16] Adjust test. --- .../druid/server/http/DataSourcesResourceTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 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 69fe23a75b5d..3af12a84ed84 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 @@ -822,15 +822,20 @@ public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() { EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); EasyMock.expect(server.getDataSource("datasource1")).andReturn(null).once(); + Interval interval = Intervals.of("2010-01-22/P1D"); + int numUpdatedSegments = + segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); + EasyMock.expect(numUpdatedSegments).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), null) + new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null) ); - Assert.assertEquals(204, response.getStatus()); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); EasyMock.verify(segmentsMetadataManager); } From 65ab201b3eabbf7fedf21d7388c2fb86425e90cb Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Thu, 14 Mar 2024 22:03:52 +0530 Subject: [PATCH 04/16] javadocs. --- .../metadata/SegmentsMetadataManager.java | 13 +++--- .../metadata/SqlSegmentsMetadataQuery.java | 40 +++++++++---------- .../SqlSegmentsMetadataManagerTest.java | 4 ++ 3 files changed, 32 insertions(+), 25 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 334cba791695..af136b446a4b 100644 --- a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java @@ -83,14 +83,17 @@ public interface SegmentsMetadataManager */ 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. + */ int markAsUnusedSegmentsInInterval(String dataSource, Interval interval); /** - * TODO fill in javadocs. - * @param dataSource - * @param interval - * @param versions List of versions - * @return + * 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); 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 384763441432..b9848d835e27 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java @@ -330,7 +330,7 @@ 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) { @@ -338,10 +338,10 @@ public int markSegmentsUnused(final String dataSource, final Interval interval) } /** - * TODO: update javadocs - * 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 filtered by an optional list of versions + * as unused. * - * @return the number of segments actually modified. + * @return Number of segments updated. */ public int markSegmentsUnused(final String dataSource, final Interval interval, @Nullable final List versions) { @@ -355,6 +355,7 @@ public int markSegmentsUnused(final String dataSource, final Interval interval, ) ); appendConditionForVersions(sb, versions); + return handle .createStatement(sb.toString()) .bind("dataSource", dataSource) @@ -375,9 +376,8 @@ public int markSegmentsUnused(final String dataSource, final Interval interval, IntervalMode.CONTAINS.makeSqlCondition(connector.getQuoteString(), ":start", ":end") ) ); - appendConditionForVersions(sb, versions); - log.info("Query[%s]", sb.toString()); + return handle .createStatement(sb.toString()) .bind("dataSource", dataSource) @@ -509,20 +509,6 @@ public static void appendConditionForIntervalsAndMatchMode( sb.append(")"); } - 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)); - } - /** * Given a Query object bind the input intervals to it * @param query Query to fetch segments @@ -878,6 +864,20 @@ 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/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java index 586268423ca7..77e811d34e2f 100644 --- a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java +++ b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java @@ -588,6 +588,10 @@ public void testMarkAsUsedNonOvershadowedSegments() throws Exception ); } + // TODO: add mark as unused by versions: + // 1. Eternity + // 2. Sane intervals + // 3. Weird/insance intervals @Test public void testMarkAsUsedNonOvershadowedSegments2() throws Exception { From c627cbe75829255687091067d67c4e37b9e7fa18 Mon Sep 17 00:00:00 2001 From: zachjsh Date: Thu, 14 Mar 2024 16:55:28 -0400 Subject: [PATCH 05/16] * fix --- .../server/http/DataSourcesResource.java | 37 ++++++++++------- .../server/http/DataSourcesResourceTest.java | 41 +++---------------- 2 files changed, 29 insertions(+), 49 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 b83bda21dcd0..d059a62071a8 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 @@ -166,12 +166,12 @@ public Response getQueryableDataSources( @Path("/{dataSourceName}") @Produces(MediaType.APPLICATION_JSON) @ResourceFilters(DatasourceResourceFilter.class) - public Response getDataSource( + public Response getQueryableDataSource( @PathParam("dataSourceName") final String dataSourceName, @QueryParam("full") final String full ) { - final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); + final ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); if (dataSource == null) { return logAndCreateDataSourceNotFoundResponse(dataSourceName); @@ -233,7 +233,13 @@ public Response markAsUsedNonOvershadowedSegments( return segmentsMetadataManager.markAsUsedNonOvershadowedSegments(dataSourceName, segmentIds); } }; - return performSegmentUpdate(dataSourceName, payload, operation); + + return performSegmentUpdate( + dataSourceName, + payload, + operation, + false // datasource may have previously had all segments marked unused, in which case it is not queryable + ); } @POST @@ -278,13 +284,14 @@ public Response markSegmentsAsUnused( ); return numUpdatedSegments; }; - return performSegmentUpdate(dataSourceName, payload, operation); + return performSegmentUpdate(dataSourceName, payload, operation, true); } private Response performSegmentUpdate( String dataSourceName, MarkDataSourceSegmentsPayload payload, - SegmentUpdateOperation operation + SegmentUpdateOperation operation, + boolean validateDatasourceIsQueryable ) { if (payload == null || !payload.isValid()) { @@ -295,9 +302,11 @@ private Response performSegmentUpdate( .build(); } - final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); - if (dataSource == null) { - return logAndCreateDataSourceNotFoundResponse(dataSourceName); + if (validateDatasourceIsQueryable) { + final ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); + if (dataSource == null) { + return logAndCreateDataSourceNotFoundResponse(dataSourceName); + } } return performSegmentUpdate(dataSourceName, operation); @@ -434,7 +443,7 @@ public Response getIntervalsWithServedSegmentsOrAllServedSegmentsPerIntervals( ) { if (simple == null && full == null) { - final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); + final ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); if (dataSource == null) { return logAndCreateDataSourceNotFoundResponse(dataSourceName); } @@ -460,7 +469,7 @@ public Response getServedSegmentsInInterval( { final Interval theInterval = Intervals.of(interval.replace('_', '/')); if (simple == null && full == null) { - final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); + final ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); if (dataSource == null) { return logAndCreateDataSourceNotFoundResponse(dataSourceName); } @@ -617,7 +626,7 @@ private Response getServedSegmentsInInterval( Predicate intervalFilter ) { - final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); + final ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); if (dataSource == null) { return logAndCreateDataSourceNotFoundResponse(dataSourceName); @@ -667,7 +676,7 @@ public Response getAllServedSegments( @QueryParam("full") String full ) { - ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); + ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); if (dataSource == null) { return logAndCreateDataSourceNotFoundResponse(dataSourceName); } @@ -689,7 +698,7 @@ public Response getServedSegment( @PathParam("segmentId") String segmentId ) { - ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); + ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); if (dataSource == null) { return logAndCreateDataSourceNotFoundResponse(dataSourceName); } @@ -747,7 +756,7 @@ public Response getTiersWhereSegmentsAreServed(@PathParam("dataSourceName") Stri } @Nullable - private ImmutableDruidDataSource getDataSource(final String dataSourceName) + private ImmutableDruidDataSource getQueryableDataSource(final String dataSourceName) { List dataSources = serverInventoryView .getInventory() 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 69fe23a75b5d..b96fbe20b553 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 @@ -320,7 +320,7 @@ public void testFullGetTheDataSource() EasyMock.replay(inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); - Response response = dataSourcesResource.getDataSource("datasource1", "full"); + Response response = dataSourcesResource.getQueryableDataSource("datasource1", "full"); ImmutableDruidDataSource result = (ImmutableDruidDataSource) response.getEntity(); Assert.assertEquals(200, response.getStatus()); ImmutableDruidDataSourceTestUtils.assertEquals(dataSource1.toImmutableDruidDataSource(), result); @@ -335,7 +335,7 @@ public void testNullGetTheDataSource() EasyMock.replay(inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); - Assert.assertEquals(204, dataSourcesResource.getDataSource("none", null).getStatus()); + Assert.assertEquals(204, dataSourcesResource.getQueryableDataSource("none", null).getStatus()); EasyMock.verify(inventoryView, server); } @@ -352,7 +352,7 @@ public void testSimpleGetTheDataSource() EasyMock.replay(inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); - Response response = dataSourcesResource.getDataSource("datasource1", null); + Response response = dataSourcesResource.getQueryableDataSource("datasource1", null); Assert.assertEquals(200, response.getStatus()); Map> result = (Map>) response.getEntity(); Assert.assertEquals(1, ((Map) (result.get("tiers").get(null))).get("segmentCount")); @@ -385,7 +385,7 @@ public void testSimpleGetTheDataSourceManyTiers() EasyMock.replay(inventoryView, server, server2, server3); DataSourcesResource dataSourcesResource = createResource(); - Response response = dataSourcesResource.getDataSource("datasource1", null); + Response response = dataSourcesResource.getQueryableDataSource("datasource1", null); Assert.assertEquals(200, response.getStatus()); Map> result = (Map>) response.getEntity(); Assert.assertEquals(2, ((Map) (result.get("tiers").get("cold"))).get("segmentCount")); @@ -423,7 +423,7 @@ public void testSimpleGetTheDataSourceWithReplicatedSegments() EasyMock.replay(inventoryView); DataSourcesResource dataSourcesResource = createResource(); - Response response = dataSourcesResource.getDataSource("datasource1", null); + Response response = dataSourcesResource.getQueryableDataSource("datasource1", null); Assert.assertEquals(200, response.getStatus()); Map> result1 = (Map>) response.getEntity(); Assert.assertEquals(2, ((Map) (result1.get("tiers").get("tier1"))).get("segmentCount")); @@ -438,7 +438,7 @@ public void testSimpleGetTheDataSourceWithReplicatedSegments() Assert.assertEquals(30L, result1.get("segments").get("size")); Assert.assertEquals(60L, result1.get("segments").get("replicatedSize")); - response = dataSourcesResource.getDataSource("datasource2", null); + response = dataSourcesResource.getQueryableDataSource("datasource2", null); Assert.assertEquals(200, response.getStatus()); Map> result2 = (Map>) response.getEntity(); Assert.assertEquals(1, ((Map) (result2.get("tiers").get("tier1"))).get("segmentCount")); @@ -733,13 +733,10 @@ public void testMarkSegmentAsUsedNoChange() @Test public void testMarkAsUsedNonOvershadowedSegmentsInterval() { - DruidDataSource dataSource = new DruidDataSource("datasource1", new HashMap<>()); Interval interval = Intervals.of("2010-01-22/P1D"); int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); EasyMock.expect(numUpdatedSegments).andReturn(3).once(); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); @@ -754,13 +751,10 @@ public void testMarkAsUsedNonOvershadowedSegmentsInterval() @Test public void testMarkAsUsedNonOvershadowedSegmentsIntervalNoneUpdated() { - DruidDataSource dataSource = new DruidDataSource("datasource1", new HashMap<>()); Interval interval = Intervals.of("2010-01-22/P1D"); int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); EasyMock.expect(numUpdatedSegments).andReturn(0).once(); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); @@ -776,13 +770,10 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalNoneUpdated() @Test public void testMarkAsUsedNonOvershadowedSegmentsSet() { - DruidDataSource dataSource = new DruidDataSource("datasource1", new HashMap<>()); Set segmentIds = ImmutableSet.of(dataSegmentList.get(1).getId().toString()); int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegments(EasyMock.eq("datasource1"), EasyMock.eq(segmentIds)); EasyMock.expect(numUpdatedSegments).andReturn(3).once(); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); @@ -798,13 +789,10 @@ public void testMarkAsUsedNonOvershadowedSegmentsSet() @Test public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() { - DruidDataSource dataSource = new DruidDataSource("datasource1", new HashMap<>()); Interval interval = Intervals.of("2010-01-22/P1D"); int numUpdatedSegments = segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); EasyMock.expect(numUpdatedSegments).andThrow(new RuntimeException("Error!")).once(); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); @@ -817,23 +805,6 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() EasyMock.verify(segmentsMetadataManager, inventoryView, server); } - @Test - public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() - { - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(null).once(); - EasyMock.replay(segmentsMetadataManager, inventoryView, server); - - DataSourcesResource dataSourcesResource = createResource(); - - Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( - "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), null) - ); - Assert.assertEquals(204, response.getStatus()); - EasyMock.verify(segmentsMetadataManager); - } - @Test public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadNoArguments() { From f451788f26c0d26379b66e9d4372a2f5f3a91c4f Mon Sep 17 00:00:00 2001 From: zachjsh Date: Fri, 15 Mar 2024 13:01:13 -0400 Subject: [PATCH 06/16] * address review comments --- .../server/http/DataSourcesResource.java | 143 ++++++++---------- .../server/http/DataSourcesResourceTest.java | 20 +++ 2 files changed, 86 insertions(+), 77 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 d059a62071a8..dcdcef22a8c8 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 @@ -209,37 +209,41 @@ public Response markAsUsedNonOvershadowedSegments( MarkDataSourceSegmentsPayload payload ) { - SegmentUpdateOperation operation = () -> { - final Interval interval = payload.getInterval(); - if (interval != null) { - return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval); - } else { - final Set segmentIds = payload.getSegmentIds(); - if (segmentIds == null || segmentIds.isEmpty()) { - return 0; - } + 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") + .build(); + } else { + SegmentUpdateOperation operation = () -> { + + final Interval interval = payload.getInterval(); + if (interval != null) { + return segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(dataSourceName, interval); + } else { + final Set segmentIds = payload.getSegmentIds(); + if (segmentIds == null || segmentIds.isEmpty()) { + return 0; + } - // Validate segmentIds - final List invalidSegmentIds = new ArrayList<>(); - for (String segmentId : segmentIds) { - if (SegmentId.iteratePossibleParsingsWithDataSource(dataSourceName, segmentId).isEmpty()) { - invalidSegmentIds.add(segmentId); + // Validate segmentIds + final List invalidSegmentIds = new ArrayList<>(); + for (String segmentId : segmentIds) { + if (SegmentId.iteratePossibleParsingsWithDataSource(dataSourceName, segmentId).isEmpty()) { + invalidSegmentIds.add(segmentId); + } + } + if (!invalidSegmentIds.isEmpty()) { + throw InvalidInput.exception("Could not parse invalid segment IDs[%s]", invalidSegmentIds); } - } - if (!invalidSegmentIds.isEmpty()) { - throw InvalidInput.exception("Could not parse invalid segment IDs[%s]", invalidSegmentIds); - } - return segmentsMetadataManager.markAsUsedNonOvershadowedSegments(dataSourceName, segmentIds); - } - }; + return segmentsMetadataManager.markAsUsedNonOvershadowedSegments(dataSourceName, segmentIds); + } + }; - return performSegmentUpdate( - dataSourceName, - payload, - operation, - false // datasource may have previously had all segments marked unused, in which case it is not queryable - ); + return performSegmentUpdate(dataSourceName, operation); + } } @POST @@ -252,47 +256,6 @@ public Response markSegmentsAsUnused( final MarkDataSourceSegmentsPayload payload, @Context final HttpServletRequest req ) - { - SegmentUpdateOperation operation = () -> { - final Interval interval = payload.getInterval(); - final int numUpdatedSegments; - if (interval != null) { - numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval); - } else { - final Set segmentIds = - payload.getSegmentIds() - .stream() - .map(idStr -> SegmentId.tryParse(dataSourceName, idStr)) - .filter(Objects::nonNull) - .collect(Collectors.toSet()); - - // Filter out segmentIds that do not belong to this datasource - numUpdatedSegments = segmentsMetadataManager.markSegmentsAsUnused( - segmentIds.stream() - .filter(segmentId -> segmentId.getDataSource().equals(dataSourceName)) - .collect(Collectors.toSet()) - ); - } - auditManager.doAudit( - AuditEntry.builder() - .key(dataSourceName) - .type("segment.markUnused") - .payload(payload) - .auditInfo(AuthorizationUtils.buildAuditInfo(req)) - .request(AuthorizationUtils.buildRequestInfo("coordinator", req)) - .build() - ); - return numUpdatedSegments; - }; - return performSegmentUpdate(dataSourceName, payload, operation, true); - } - - private Response performSegmentUpdate( - String dataSourceName, - MarkDataSourceSegmentsPayload payload, - SegmentUpdateOperation operation, - boolean validateDatasourceIsQueryable - ) { if (payload == null || !payload.isValid()) { log.warn("Invalid request payload: [%s]", payload); @@ -300,16 +263,42 @@ private Response performSegmentUpdate( .status(Response.Status.BAD_REQUEST) .entity("Invalid request payload, either interval or segmentIds array must be specified") .build(); + } else if (getQueryableDataSource(dataSourceName) == null) { + return logAndCreateDataSourceNotFoundResponse(dataSourceName); + } else { + SegmentUpdateOperation operation = () -> { + final Interval interval = payload.getInterval(); + final int numUpdatedSegments; + if (interval != null) { + numUpdatedSegments = segmentsMetadataManager.markAsUnusedSegmentsInInterval(dataSourceName, interval); + } else { + final Set segmentIds = + payload.getSegmentIds() + .stream() + .map(idStr -> SegmentId.tryParse(dataSourceName, idStr)) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + // Filter out segmentIds that do not belong to this datasource + numUpdatedSegments = segmentsMetadataManager.markSegmentsAsUnused( + segmentIds.stream() + .filter(segmentId -> segmentId.getDataSource().equals(dataSourceName)) + .collect(Collectors.toSet()) + ); + } + auditManager.doAudit( + AuditEntry.builder() + .key(dataSourceName) + .type("segment.markUnused") + .payload(payload) + .auditInfo(AuthorizationUtils.buildAuditInfo(req)) + .request(AuthorizationUtils.buildRequestInfo("coordinator", req)) + .build() + ); + return numUpdatedSegments; + }; + return performSegmentUpdate(dataSourceName, operation); } - - if (validateDatasourceIsQueryable) { - final ImmutableDruidDataSource dataSource = getQueryableDataSource(dataSourceName); - if (dataSource == null) { - return logAndCreateDataSourceNotFoundResponse(dataSourceName); - } - } - - return performSegmentUpdate(dataSourceName, operation); } private static Response logAndCreateDataSourceNotFoundResponse(String dataSourceName) 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 b96fbe20b553..8d5717d6db3f 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 @@ -805,6 +805,26 @@ public void testMarkAsUsedNonOvershadowedSegmentsIntervalException() EasyMock.verify(segmentsMetadataManager, inventoryView, server); } + @Test + public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() + { + Interval interval = Intervals.of("2010-01-22/P1D"); + int numUpdatedSegments = + segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); + EasyMock.expect(numUpdatedSegments).andReturn(0).once(); + EasyMock.replay(segmentsMetadataManager, inventoryView, server); + + DataSourcesResource dataSourcesResource = createResource(); + + Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( + "datasource1", + new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), null) + ); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), response.getEntity()); + EasyMock.verify(segmentsMetadataManager); + } + @Test public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadNoArguments() { From 9e294831fafefd397ee657f05a313299aa7830e0 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 10:14:08 -0700 Subject: [PATCH 07/16] Add some tests and cleanup code. --- .../metadata/SqlSegmentsMetadataManager.java | 9 +- .../SqlSegmentsMetadataManagerTest.java | 117 ++++++++++++++++-- 2 files changed, 107 insertions(+), 19 deletions(-) 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 4e90edcddb01..cb646f156fcf 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java @@ -672,10 +672,6 @@ public int markAsUsedNonOvershadowedSegmentsInInterval(final String dataSource, return doMarkAsUsedNonOvershadowedSegments(dataSource, interval, versions); } - /** - * Implementation for both {@link #markAsUsedAllNonOvershadowedSegmentsInDataSource} (if the given interval is null) - * and {@link #markAsUsedNonOvershadowedSegmentsInInterval}. - */ private int doMarkAsUsedNonOvershadowedSegments( final String dataSourceName, final @Nullable Interval interval, @@ -712,7 +708,6 @@ private int doMarkAsUsedNonOvershadowedSegments( } ); - log.info("Unused segments[%s] to be marked as used", unusedSegments); return markNonOvershadowedSegmentsAsUsed(unusedSegments, timeline); } @@ -884,13 +879,13 @@ public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interv } @Override - public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interval, List versions) + public int markAsUnusedSegmentsInInterval(final String dataSource, final Interval interval, final List versions) { try { return connector.getDBI().withHandle( handle -> SqlSegmentsMetadataQuery.forHandle(handle, connector, dbTables.get(), jsonMapper) - .markSegmentsUnused(dataSourceName, interval, versions) + .markSegmentsUnused(dataSource, interval, versions) ); } catch (Exception e) { 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 77e811d34e2f..1874886832e6 100644 --- a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java +++ b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java @@ -588,12 +588,8 @@ public void testMarkAsUsedNonOvershadowedSegments() throws Exception ); } - // TODO: add mark as unused by versions: - // 1. Eternity - // 2. Sane intervals - // 3. Weird/insance intervals @Test - public void testMarkAsUsedNonOvershadowedSegments2() throws Exception + public void testMarkAsUsedNonOvershadowedSegmentsInEternityIntervalWithVersions() throws Exception { publishWikiSegments(); sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); @@ -620,11 +616,56 @@ public void testMarkAsUsedNonOvershadowedSegments2() throws Exception ); publishUnusedSegments(koalaSegment1, koalaSegment2, koalaSegment3); - final Set segmentIds = ImmutableSet.of( - koalaSegment1.getId().toString(), - koalaSegment2.getId().toString(), - koalaSegment3.getId().toString() + + 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( @@ -633,10 +674,12 @@ public void testMarkAsUsedNonOvershadowedSegments2() throws Exception ); Assert.assertEquals( 2, -// sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(DS.KOALA, Intervals.ETERNITY, ImmutableList.of("2017-10-15T20:19:12.565Z", "2017-10-16T20:19:12.565Z")) - sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(DS.KOALA, Intervals.ETERNITY, ImmutableList.of("foo", "bar")) + 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") + ) ); -// Assert.assertEquals(2, sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegments(DS.KOALA, segmentIds)); sqlSegmentsMetadataManager.poll(); Assert.assertEquals( @@ -645,6 +688,56 @@ public void testMarkAsUsedNonOvershadowedSegments2() throws Exception ); } + @Test + public void testMarkAsUsedNonOvershadowedSegmentsWithInvalidVersions() 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 { From 67e2c7bd1d193053f7fae1974c69c38d5b362b98 Mon Sep 17 00:00:00 2001 From: zachjsh Date: Fri, 15 Mar 2024 13:18:24 -0400 Subject: [PATCH 08/16] * all remove the short-circuit for markUnused api --- .../druid/server/http/DataSourcesResource.java | 2 -- .../server/http/DataSourcesResourceTest.java | 15 --------------- 2 files changed, 17 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 dcdcef22a8c8..d0458243fcfc 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 @@ -263,8 +263,6 @@ public Response markSegmentsAsUnused( .status(Response.Status.BAD_REQUEST) .entity("Invalid request payload, either interval or segmentIds array must be specified") .build(); - } else if (getQueryableDataSource(dataSourceName) == null) { - return logAndCreateDataSourceNotFoundResponse(dataSourceName); } else { SegmentUpdateOperation operation = () -> { final Interval interval = payload.getInterval(); 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 8d5717d6db3f..aff5af378fdf 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 @@ -1010,8 +1010,6 @@ public void testMarkSegmentsAsUnused() .map(DataSegment::getId) .collect(Collectors.toSet()); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource1).once(); EasyMock.expect(segmentsMetadataManager.markSegmentsAsUnused(segmentIds)).andReturn(1).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -1041,8 +1039,6 @@ public void testMarkSegmentsAsUnusedNoChanges() .map(DataSegment::getId) .collect(Collectors.toSet()); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource1).once(); EasyMock.expect(segmentsMetadataManager.markSegmentsAsUnused(segmentIds)).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -1072,8 +1068,6 @@ public void testMarkSegmentsAsUnusedException() .map(DataSegment::getId) .collect(Collectors.toSet()); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource1).once(); EasyMock.expect(segmentsMetadataManager.markSegmentsAsUnused(segmentIds)) .andThrow(new RuntimeException("Exception occurred")) .once(); @@ -1099,10 +1093,7 @@ public void testMarkSegmentsAsUnusedException() public void testMarkAsUnusedSegmentsInInterval() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - final DruidDataSource dataSource1 = new DruidDataSource("datasource1", new HashMap<>()); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource1).once(); EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)).andReturn(1).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -1122,10 +1113,7 @@ public void testMarkAsUnusedSegmentsInInterval() public void testMarkAsUnusedSegmentsInIntervalNoChanges() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - final DruidDataSource dataSource1 = new DruidDataSource("datasource1", new HashMap<>()); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource1).once(); EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); @@ -1144,10 +1132,7 @@ public void testMarkAsUnusedSegmentsInIntervalNoChanges() public void testMarkAsUnusedSegmentsInIntervalException() { final Interval theInterval = Intervals.of("2010-01-01/P1D"); - final DruidDataSource dataSource1 = new DruidDataSource("datasource1", new HashMap<>()); - EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); - EasyMock.expect(server.getDataSource("datasource1")).andReturn(dataSource1).once(); EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)) .andThrow(new RuntimeException("Exception occurred")) .once(); From acda30704e6e40ffcc65e7147c4df53d0578bc14 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 10:29:11 -0700 Subject: [PATCH 09/16] Update tests. --- .../metadata/SqlSegmentsMetadataManager.java | 12 ++- .../SqlSegmentsMetadataManagerTest.java | 79 +++++++++++++++++-- 2 files changed, 82 insertions(+), 9 deletions(-) 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 cb646f156fcf..f3f782f50e5f 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java @@ -665,7 +665,11 @@ public int markAsUsedNonOvershadowedSegmentsInInterval(final String dataSource, } @Override - public int markAsUsedNonOvershadowedSegmentsInInterval(final String dataSource, final Interval interval, final List versions) + public int markAsUsedNonOvershadowedSegmentsInInterval( + final String dataSource, + final Interval interval, + @Nullable final List versions + ) { Preconditions.checkNotNull(interval); Preconditions.checkNotNull(versions); @@ -879,7 +883,11 @@ public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interv } @Override - public int markAsUnusedSegmentsInInterval(final String dataSource, final Interval interval, final List versions) + public int markAsUnusedSegmentsInInterval( + final String dataSource, + final Interval interval, + @Nullable final List versions + ) { try { return connector.getDBI().withHandle( 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 0a8f678c1d34..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; @@ -690,7 +691,7 @@ public void testMarkAsUsedNonOvershadowedSegmentsInFiniteIntervalWithVersions() } @Test - public void testMarkAsUsedNonOvershadowedSegmentsWithInvalidVersions() throws Exception + public void testMarkAsUsedNonOvershadowedSegmentsWithNonExistentVersions() throws Exception { publishWikiSegments(); sqlSegmentsMetadataManager.startPollingDatabasePeriodically(); @@ -954,23 +955,38 @@ public void testMarkAsUnusedSegmentsInIntervalAndVersions() throws IOException sqlSegmentsMetadataManager.poll(); Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically()); - final DataSegment koalaSegment1 = createNewSegment1(DS.KOALA); - final DataSegment koalaSegment2 = createNewSegment2(DS.KOALA); + 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", - "2017-10-15T20:19:12.565Z" + v2 ); publisher.publishSegment(koalaSegment1); publisher.publishSegment(koalaSegment2); publisher.publishSegment(koalaSegment3); - final Interval theInterval = Intervals.of("2017-10-15T00:00:00.000/2017-10-18T00:00:00.000"); + final Interval theInterval = Intervals.of("2017-10-15/2017-10-18"); - // 2 out of 3 segments match the interval Assert.assertEquals( 2, - sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(DS.KOALA, theInterval, ImmutableList.of("foo", "bar", "2017-10-15T20:19:12.565Z")) + sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval( + DS.KOALA, + theInterval, + ImmutableList.of(v1, v2) + ) ); sqlSegmentsMetadataManager.poll(); @@ -980,6 +996,55 @@ public void testMarkAsUnusedSegmentsInIntervalAndVersions() throws IOException ); } + @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 { From 45f00fca95a6865d12ab20d7a0ff0f3b4c1fdc2c Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 10:31:23 -0700 Subject: [PATCH 10/16] Revert "Merge branch 'fix_mark_used_interval_when_no_segments' into mark_used_unused_api_versions" This reverts commit d06ccf589abdc69cc5aacb7e4654830c098f0157, reversing changes made to 8189b54115cb4657fda77ac7fda762c93de30753. --- .../druid/server/http/DataSourcesResource.java | 14 ++++++-------- .../druid/server/http/DataSourcesResourceTest.java | 9 ++------- 2 files changed, 8 insertions(+), 15 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 e1d1bef2d6b2..57e636b9ceb9 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 @@ -238,7 +238,7 @@ public Response markAsUsedNonOvershadowedSegments( return segmentsMetadataManager.markAsUsedNonOvershadowedSegments(dataSourceName, segmentIds); } }; - return performSegmentUpdate(dataSourceName, payload, operation, true); + return performSegmentUpdate(dataSourceName, payload, operation); } @POST @@ -290,14 +290,13 @@ public Response markSegmentsAsUnused( ); return numUpdatedSegments; }; - return performSegmentUpdate(dataSourceName, payload, operation, false); + return performSegmentUpdate(dataSourceName, payload, operation); } private Response performSegmentUpdate( String dataSourceName, MarkDataSourceSegmentsPayload payload, - SegmentUpdateOperation operation, - boolean isMarkUsed + SegmentUpdateOperation operation ) { if (payload == null || !payload.isValid()) { @@ -309,10 +308,9 @@ private Response performSegmentUpdate( } final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); - // We can skip the markUnused operation when there are no used segments for this datasource. - // However, for markUsed operation, we should still continue as we want to mark a portion of the unused segments as used. - if (dataSource == null && !isMarkUsed) { - return logAndCreateDataSourceNotFoundResponse(dataSourceName); + if (dataSource == null) { + log.info("Datasource[%s] not found, but perhaps there are no used segments.", dataSourceName); +// return logAndCreateDataSourceNotFoundResponse(dataSourceName); } return performSegmentUpdate(dataSourceName, operation); 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 6d21844edf5c..6005ef8ea7ac 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 @@ -822,20 +822,15 @@ public void testMarkAsUsedNonOvershadowedSegmentsNoDataSource() { EasyMock.expect(inventoryView.getInventory()).andReturn(ImmutableList.of(server)).once(); EasyMock.expect(server.getDataSource("datasource1")).andReturn(null).once(); - Interval interval = Intervals.of("2010-01-22/P1D"); - int numUpdatedSegments = - segmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(EasyMock.eq("datasource1"), EasyMock.eq(interval)); - EasyMock.expect(numUpdatedSegments).andReturn(0).once(); EasyMock.replay(segmentsMetadataManager, inventoryView, server); DataSourcesResource dataSourcesResource = createResource(); Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments( "datasource1", - new DataSourcesResource.MarkDataSourceSegmentsPayload(interval, null, 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()); + Assert.assertEquals(204, response.getStatus()); EasyMock.verify(segmentsMetadataManager); } From 2c3433545c5155e5cad5efcf5f2949638ef47cf3 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 10:38:42 -0700 Subject: [PATCH 11/16] revert one more change in favor of PR #16127 --- .../java/org/apache/druid/server/http/DataSourcesResource.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 57e636b9ceb9..416f126bee9b 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 @@ -309,8 +309,7 @@ private Response performSegmentUpdate( final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); if (dataSource == null) { - log.info("Datasource[%s] not found, but perhaps there are no used segments.", dataSourceName); -// return logAndCreateDataSourceNotFoundResponse(dataSourceName); + return logAndCreateDataSourceNotFoundResponse(dataSourceName); } return performSegmentUpdate(dataSourceName, operation); From d848ef13174ad5b53d7a681e717224e25e8abd9f Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 10:40:38 -0700 Subject: [PATCH 12/16] checkstyle --- .../org/apache/druid/metadata/SqlSegmentsMetadataQuery.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 b9848d835e27..2164e1ff5d4e 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java @@ -867,7 +867,8 @@ private static int computeNumChangedSegments(List segmentIds, int[] segm private static void appendConditionForVersions( final StringBuilder sb, final List versions - ) { + ) + { if (CollectionUtils.isNullOrEmpty(versions)) { return; } From c44eaa89b93cd3147ff0f05f92a7a558bee64884 Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 10:53:29 -0700 Subject: [PATCH 13/16] adjust nullables and add javadocs. --- .../apache/druid/metadata/SegmentsMetadataManager.java | 10 ++++++++++ .../druid/metadata/SqlSegmentsMetadataManager.java | 7 +++++-- .../druid/metadata/SqlSegmentsMetadataQuery.java | 7 ++----- 3 files changed, 17 insertions(+), 7 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 af136b446a4b..1adbb05a716a 100644 --- a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java @@ -53,8 +53,17 @@ public interface SegmentsMetadataManager */ int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource); + /** + * Marks non-overshadowed unused segments for the given interval as used. + * @return Number of segments updated + */ int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval); + /** + * Marks non-overshadowed unused segments for the given interval and versions as used. + * {@code versions} is required. For version agnostic API, use {@link #markAsUsedNonOvershadowedSegmentsInInterval}. + * @return Number of segments updated + */ int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions); /** @@ -93,6 +102,7 @@ public interface SegmentsMetadataManager /** * 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. + * {@code versions} is required. For version agnostic API, use {@link #markAsUnusedSegmentsInInterval}. * @return The number of segments updated. */ int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions); 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 f3f782f50e5f..be7c4e4b504e 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java @@ -668,7 +668,7 @@ public int markAsUsedNonOvershadowedSegmentsInInterval(final String dataSource, public int markAsUsedNonOvershadowedSegmentsInInterval( final String dataSource, final Interval interval, - @Nullable final List versions + final List versions ) { Preconditions.checkNotNull(interval); @@ -870,6 +870,7 @@ public int markSegmentsAsUnused(Set segmentIds) @Override public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interval) { + Preconditions.checkNotNull(interval); try { return connector.getDBI().withHandle( handle -> @@ -886,9 +887,11 @@ public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interv public int markAsUnusedSegmentsInInterval( final String dataSource, final Interval interval, - @Nullable final List versions + final List versions ) { + Preconditions.checkNotNull(interval); + Preconditions.checkNotNull(versions); try { return connector.getDBI().withHandle( handle -> 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 2164e1ff5d4e..49f6a7de5734 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java @@ -124,11 +124,8 @@ public CloseableIterator retrieveUsedSegments( } /** - * TODO: javadocs - * @param dataSource - * @param intervals - * @param versions - * @return + * 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, From 4a965c931ec206accbc1f6fc28925f6ceadb55f1 Mon Sep 17 00:00:00 2001 From: zachjsh Date: Fri, 15 Mar 2024 14:42:19 -0400 Subject: [PATCH 14/16] * add test --- .../server/http/DataSourcesResourceTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 aff5af378fdf..ace68ad3c21e 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 @@ -1149,6 +1149,24 @@ public void testMarkAsUnusedSegmentsInIntervalException() EasyMock.verify(segmentsMetadataManager, inventoryView, server); } + @Test + public void testMarkAsUnusedSegmentsInIntervalNoDataSource() + { + final Interval theInterval = Intervals.of("2010-01-01/P1D"); + EasyMock.expect(segmentsMetadataManager.markAsUnusedSegmentsInInterval("datasource1", theInterval)).andReturn(0).once(); + EasyMock.replay(segmentsMetadataManager, inventoryView, server); + + final DataSourcesResource.MarkDataSourceSegmentsPayload payload = + new DataSourcesResource.MarkDataSourceSegmentsPayload(theInterval, null); + 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() { From cbd09cd4acd449755f1f3fe2d0b1f1ac8285f4dc Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 14:47:32 -0700 Subject: [PATCH 15/16] Comment fix up. --- .../test/java/org/apache/druid/metadata/TestDerbyConnector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 { From 8cd8200e2996a6c09afa220ea36469b98e98465b Mon Sep 17 00:00:00 2001 From: Abhishek Balaji Radhakrishnan Date: Fri, 15 Mar 2024 14:55:32 -0700 Subject: [PATCH 16/16] default method and remove some unnecessary overridden code. --- .../metadata/SegmentsMetadataManager.java | 18 +++++++----- .../metadata/SqlSegmentsMetadataManager.java | 29 ++----------------- .../simulate/TestSegmentsMetadataManager.java | 12 -------- 3 files changed, 13 insertions(+), 46 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 1adbb05a716a..91e33b7a231c 100644 --- a/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java @@ -57,11 +57,13 @@ public interface SegmentsMetadataManager * Marks non-overshadowed unused segments for the given interval as used. * @return Number of segments updated */ - int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval); + default int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval) + { + return markAsUsedNonOvershadowedSegmentsInInterval(dataSource, interval, null); + } /** - * Marks non-overshadowed unused segments for the given interval and versions as used. - * {@code versions} is required. For version agnostic API, use {@link #markAsUsedNonOvershadowedSegmentsInInterval}. + * 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); @@ -95,15 +97,17 @@ public interface SegmentsMetadataManager /** * 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. + * @return Number of segments updated */ - int markAsUnusedSegmentsInInterval(String dataSource, Interval interval); + 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. - * {@code versions} is required. For version agnostic API, use {@link #markAsUnusedSegmentsInInterval}. - * @return The number of segments updated. + * @return The number of segments updated */ int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions); 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 be7c4e4b504e..66a60e072c02 100644 --- a/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java @@ -657,22 +657,14 @@ public int markAsUsedAllNonOvershadowedSegmentsInDataSource(final String dataSou return doMarkAsUsedNonOvershadowedSegments(dataSource, null, null); } - @Override - public int markAsUsedNonOvershadowedSegmentsInInterval(final String dataSource, final Interval interval) - { - Preconditions.checkNotNull(interval); - return doMarkAsUsedNonOvershadowedSegments(dataSource, interval, null); - } - @Override public int markAsUsedNonOvershadowedSegmentsInInterval( final String dataSource, final Interval interval, - final List versions + @Nullable final List versions ) { Preconditions.checkNotNull(interval); - Preconditions.checkNotNull(versions); return doMarkAsUsedNonOvershadowedSegments(dataSource, interval, versions); } @@ -867,31 +859,14 @@ public int markSegmentsAsUnused(Set segmentIds) ); } - @Override - public int markAsUnusedSegmentsInInterval(String dataSourceName, Interval interval) - { - Preconditions.checkNotNull(interval); - try { - return connector.getDBI().withHandle( - handle -> - SqlSegmentsMetadataQuery.forHandle(handle, connector, dbTables.get(), jsonMapper) - .markSegmentsUnused(dataSourceName, interval) - ); - } - catch (Exception e) { - throw new RuntimeException(e); - } - } - @Override public int markAsUnusedSegmentsInInterval( final String dataSource, final Interval interval, - final List versions + @Nullable final List versions ) { Preconditions.checkNotNull(interval); - Preconditions.checkNotNull(versions); try { return connector.getDBI().withHandle( handle -> 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 53f51b3d14a0..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 @@ -86,12 +86,6 @@ public int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource) return 0; } - @Override - public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval) - { - return 0; - } - @Override public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval, List versions) { @@ -121,12 +115,6 @@ public int markAsUnusedAllSegmentsInDataSource(String dataSource) return 0; } - @Override - public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval) - { - return 0; - } - @Override public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval, List versions) {