From ca33d8bf2e131801318b0c170b7e312669443026 Mon Sep 17 00:00:00 2001 From: rishabh singh Date: Thu, 14 Mar 2024 16:49:41 +0530 Subject: [PATCH 1/6] Fix build --- .../org/apache/druid/sql/calcite/CalciteArraysQueryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index 141baa5e5308..2c165ffe3c3b 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -1160,7 +1160,7 @@ public void testArrayContainsArrayStringColumns() "SELECT ARRAY_CONTAINS(arrayStringNulls, ARRAY['a', 'b']), ARRAY_CONTAINS(arrayStringNulls, arrayString) FROM druid.arrays LIMIT 5", ImmutableList.of( newScanQueryBuilder() - .dataSource(DATA_SOURCE_ARRAYS) + .dataSource(CalciteTests.ARRAYS_DATASOURCE) .intervals(querySegmentSpec(Filtration.eternity())) .columns("v0", "v1") .virtualColumns( From 6e36e2c27ec83e694804835bd7764addc39ba816 Mon Sep 17 00:00:00 2001 From: rishabh singh Date: Mon, 12 Aug 2024 22:04:43 +0530 Subject: [PATCH 2/6] Support segment metadata queries for tombstones --- .../loading/TombstoneSegmentizerFactory.java | 34 +++++++++++++++---- .../apache/druid/client/BrokerServerView.java | 2 +- .../druid/client/CachingClusteredClient.java | 2 +- .../server/coordination/ServerManager.java | 8 ++--- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java b/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java index 72958e8049d3..e8d63ec899a2 100644 --- a/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java @@ -30,14 +30,17 @@ import org.apache.druid.segment.SegmentLazyLoadFailCallback; import org.apache.druid.segment.StorageAdapter; import org.apache.druid.segment.column.ColumnHolder; +import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.data.Indexed; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; +import org.joda.time.DateTime; import org.joda.time.Interval; import javax.annotation.Nullable; - import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -74,13 +77,13 @@ public Interval getDataInterval() @Override public int getNumRows() { - throw new UnsupportedOperationException(); + return 0; } @Override public Indexed getAvailableDimensions() { - throw new UnsupportedOperationException(); + return Indexed.empty(); } @Override @@ -93,13 +96,13 @@ public BitmapFactory getBitmapFactoryForDimensions() @Override public Metadata getMetadata() { - throw new UnsupportedOperationException(); + return null; } @Override public Map getDimensionHandlers() { - throw new UnsupportedOperationException(); + return new HashMap<>(); } @Override @@ -111,7 +114,7 @@ public void close() @Override public List getColumnNames() { - throw new UnsupportedOperationException(); + return new ArrayList<>(); } @Nullable @@ -120,7 +123,6 @@ public ColumnHolder getColumnHolder(String columnName) { throw new UnsupportedOperationException(); } - }; final QueryableIndexStorageAdapter storageAdapter = new QueryableIndexStorageAdapter(queryableIndex) @@ -130,6 +132,24 @@ public boolean isFromTombstone() { return true; } + + @Override + public DateTime getMinTime() + { + return tombstone.getInterval().getStart(); + } + + @Override + public DateTime getMaxTime() + { + return tombstone.getInterval().getEnd(); + } + + @Override + public RowSignature getRowSignature() + { + return RowSignature.builder().build(); + } }; Segment segmentObject = new Segment() diff --git a/server/src/main/java/org/apache/druid/client/BrokerServerView.java b/server/src/main/java/org/apache/druid/client/BrokerServerView.java index 2cb2bec03b59..cd95b4b73d6e 100644 --- a/server/src/main/java/org/apache/druid/client/BrokerServerView.java +++ b/server/src/main/java/org/apache/druid/client/BrokerServerView.java @@ -254,7 +254,7 @@ private void serverAddedSegment(final DruidServerMetadata server, final DataSegm VersionedIntervalTimeline timeline = timelines.get(segment.getDataSource()); if (timeline == null) { // broker needs to skip tombstones - timeline = new VersionedIntervalTimeline<>(Ordering.natural(), true); + timeline = new VersionedIntervalTimeline<>(Ordering.natural(), false); timelines.put(segment.getDataSource(), timeline); } diff --git a/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java b/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java index 5fa34d6699d8..3f0e7a51edf6 100644 --- a/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java +++ b/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java @@ -882,7 +882,7 @@ public TimelineLookup apply(TimelineLookup timeline) Objects::nonNull ); final VersionedIntervalTimeline newTimeline = - new VersionedIntervalTimeline<>(Ordering.natural(), true); + new VersionedIntervalTimeline<>(Ordering.natural(), false); // VersionedIntervalTimeline#addAll implementation is much more efficient than calling VersionedIntervalTimeline#add // in a loop when there are lot of segments to be added for same interval and version. newTimeline.addAll(iterator); diff --git a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java index b479737c9940..5674d35da8ed 100644 --- a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java +++ b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java @@ -260,6 +260,7 @@ protected QueryRunner buildQueryRunnerForSegment( final ReferenceCountingSegment segment = chunk.getObject(); return buildAndDecorateQueryRunner( + query.getType(), factory, toolChest, segmentMapFn.apply(segment), @@ -270,6 +271,7 @@ protected QueryRunner buildQueryRunnerForSegment( } private QueryRunner buildAndDecorateQueryRunner( + final String queryType, final QueryRunnerFactory> factory, final QueryToolChest> toolChest, final SegmentReference segment, @@ -278,9 +280,6 @@ private QueryRunner buildAndDecorateQueryRunner( final AtomicLong cpuTimeAccumulator ) { - - - final SpecificSegmentSpec segmentSpec = new SpecificSegmentSpec(segmentDescriptor); final SegmentId segmentId = segment.getId(); final Interval segmentInterval = segment.getDataInterval(); @@ -294,9 +293,10 @@ private QueryRunner buildAndDecorateQueryRunner( StorageAdapter storageAdapter = segment.asStorageAdapter(); // Short-circuit when the index comes from a tombstone (it has no data by definition), // check for null also since no all segments (higher level ones) will have QueryableIndex... - if (storageAdapter.isFromTombstone()) { + if (storageAdapter.isFromTombstone() && queryType.equals(Query.SEGMENT_METADATA)) { return new NoopQueryRunner<>(); } + String segmentIdString = segmentId.toString(); MetricsEmittingQueryRunner metricsEmittingQueryRunnerInner = new MetricsEmittingQueryRunner<>( From 433b19c97dfae9d7e38a7b8bd9740ca788ae0f25 Mon Sep 17 00:00:00 2001 From: rishabh singh Date: Tue, 13 Aug 2024 20:49:02 +0530 Subject: [PATCH 3/6] Filter out tombstone segments from metadata cache --- .../AbstractSegmentMetadataCache.java | 11 +++ .../CoordinatorSegmentMetadataCacheTest.java | 73 +++++++++++++++++++ .../BrokerSegmentMetadataCacheTest.java | 72 ++++++++++++++++++ 3 files changed, 156 insertions(+) diff --git a/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java b/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java index 88e6ee97b983..fbe3a7c7839f 100644 --- a/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java +++ b/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java @@ -458,6 +458,13 @@ public int getTotalSegments() @VisibleForTesting public void addSegment(final DruidServerMetadata server, final DataSegment segment) { + // Skip adding tombstone segment to the cache, as they do not have any data or column + // If they are added, the cache tries to refresh these segments indefinitely + // since segment metadata queries doesn't reflect metadata in tombstones. + // Refer: https://github.com/apache/druid/pull/12137 + if (segment.isTombstone()) { + return; + } // Get lock first so that we won't wait in ConcurrentMap.compute(). synchronized (lock) { // someday we could hypothetically remove broker special casing, whenever BrokerServerView supports tracking @@ -530,6 +537,10 @@ public void addSegment(final DruidServerMetadata server, final DataSegment segme @VisibleForTesting public void removeSegment(final DataSegment segment) { + // tombstone segments are not present in the cache + if (segment.isTombstone()) { + return; + } // Get lock first so that we won't wait in ConcurrentMap.compute(). synchronized (lock) { log.debug("Segment [%s] is gone.", segment.getId()); diff --git a/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java b/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java index ef1fb1e8eddf..42e8fc3fbebd 100644 --- a/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java +++ b/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java @@ -64,6 +64,7 @@ import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.incremental.IncrementalIndexSchema; +import org.apache.druid.segment.loading.LoadSpec; import org.apache.druid.segment.realtime.appenderator.SegmentSchemas; import org.apache.druid.segment.writeout.OffHeapMemorySegmentWriteOutMediumFactory; import org.apache.druid.server.QueryLifecycle; @@ -80,6 +81,8 @@ import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.LinearShardSpec; +import org.apache.druid.timeline.partition.NumberedShardSpec; +import org.apache.druid.timeline.partition.TombstoneShardSpec; import org.easymock.EasyMock; import org.joda.time.Period; import org.junit.After; @@ -2160,6 +2163,76 @@ public void testColdDatasourceSchema_verifyStaleDatasourceRemoved() Assert.assertEquals(Collections.singleton("beta"), schema.getDataSourceInformationMap().keySet()); } + @Test + public void testTombstoneSegmentIsNotAdded() throws InterruptedException + { + String datasource = "newSegmentAddTest"; + CountDownLatch addSegmentLatch = new CountDownLatch(1); + CoordinatorSegmentMetadataCache schema = new CoordinatorSegmentMetadataCache( + getQueryLifecycleFactory(walker), + serverView, + SEGMENT_CACHE_CONFIG_DEFAULT, + new NoopEscalator(), + new InternalQueryConfig(), + new NoopServiceEmitter(), + segmentSchemaCache, + backFillQueue, + sqlSegmentsMetadataManager, + segmentsMetadataManagerConfigSupplier + ) + { + @Override + public void addSegment(final DruidServerMetadata server, final DataSegment segment) + { + super.addSegment(server, segment); + if (datasource.equals(segment.getDataSource())) { + addSegmentLatch.countDown(); + } + } + }; + + schema.onLeaderStart(); + schema.awaitInitialization(); + + DataSegment segment = new DataSegment( + datasource, + Intervals.of("2001/2002"), + "1", + Collections.emptyMap(), + Collections.emptyList(), + Collections.emptyList(), + TombstoneShardSpec.INSTANCE, + null, + null, + 0 + ); + + Assert.assertEquals(6, schema.getTotalSegments()); + + serverView.addSegment(segment, ServerType.HISTORICAL); + Assert.assertTrue(addSegmentLatch.await(1, TimeUnit.SECONDS)); + Assert.assertEquals(0, addSegmentLatch.getCount()); + + Assert.assertEquals(6, schema.getTotalSegments()); + List metadatas = schema + .getSegmentMetadataSnapshot() + .values() + .stream() + .filter(metadata -> datasource.equals(metadata.getSegment().getDataSource())) + .collect(Collectors.toList()); + Assert.assertEquals(0, metadatas.size()); + + serverView.removeSegment(segment, ServerType.HISTORICAL); + Assert.assertEquals(6, schema.getTotalSegments()); + metadatas = schema + .getSegmentMetadataSnapshot() + .values() + .stream() + .filter(metadata -> datasource.equals(metadata.getSegment().getDataSource())) + .collect(Collectors.toList()); + Assert.assertEquals(0, metadatas.size()); + } + private void verifyFooDSSchema(CoordinatorSegmentMetadataCache schema, int columns) { final DataSourceInformation fooDs = schema.getDatasource("foo"); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java index 65610ce99f28..e22621b2bfb5 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java @@ -55,11 +55,13 @@ import org.apache.druid.segment.QueryableIndex; import org.apache.druid.segment.QueryableIndexStorageAdapter; import org.apache.druid.segment.TestHelper; +import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.incremental.IncrementalIndexSchema; import org.apache.druid.segment.metadata.AbstractSegmentMetadataCache; import org.apache.druid.segment.metadata.AvailableSegmentMetadata; import org.apache.druid.segment.metadata.CentralizedDatasourceSchemaConfig; +import org.apache.druid.segment.metadata.CoordinatorSegmentMetadataCache; import org.apache.druid.segment.metadata.DataSourceInformation; import org.apache.druid.segment.writeout.OffHeapMemorySegmentWriteOutMediumFactory; import org.apache.druid.server.QueryLifecycle; @@ -79,6 +81,7 @@ import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.LinearShardSpec; import org.apache.druid.timeline.partition.NumberedShardSpec; +import org.apache.druid.timeline.partition.TombstoneShardSpec; import org.easymock.EasyMock; import org.joda.time.Period; import org.junit.After; @@ -1136,4 +1139,73 @@ public void testNoDatasourceSchemaWhenNoSegmentMetadata() throws InterruptedExce Assert.assertNull(schema.getDatasource("foo")); } + + @Test + public void testTombstoneSegmentIsNotAdded() throws InterruptedException + { + String datasource = "newSegmentAddTest"; + CountDownLatch addSegmentLatch = new CountDownLatch(1); + BrokerSegmentMetadataCache schema = new BrokerSegmentMetadataCache( + CalciteTests.createMockQueryLifecycleFactory(walker, conglomerate), + serverView, + BrokerSegmentMetadataCacheConfig.create(), + new NoopEscalator(), + new InternalQueryConfig(), + new NoopServiceEmitter(), + new PhysicalDatasourceMetadataFactory(globalTableJoinable, segmentManager), + new NoopCoordinatorClient(), + CentralizedDatasourceSchemaConfig.create() + ) + { + @Override + public void addSegment(final DruidServerMetadata server, final DataSegment segment) + { + super.addSegment(server, segment); + if (datasource.equals(segment.getDataSource())) { + addSegmentLatch.countDown(); + } + } + }; + + schema.start(); + schema.awaitInitialization(); + + DataSegment segment = new DataSegment( + datasource, + Intervals.of("2001/2002"), + "1", + Collections.emptyMap(), + Collections.emptyList(), + Collections.emptyList(), + TombstoneShardSpec.INSTANCE, + null, + null, + 0 + ); + + Assert.assertEquals(6, schema.getTotalSegments()); + + serverView.addSegment(segment, ServerType.HISTORICAL); + Assert.assertTrue(addSegmentLatch.await(1, TimeUnit.SECONDS)); + Assert.assertEquals(0, addSegmentLatch.getCount()); + + Assert.assertEquals(6, schema.getTotalSegments()); + List metadatas = schema + .getSegmentMetadataSnapshot() + .values() + .stream() + .filter(metadata -> datasource.equals(metadata.getSegment().getDataSource())) + .collect(Collectors.toList()); + Assert.assertEquals(0, metadatas.size()); + + serverView.removeSegment(segment, ServerType.HISTORICAL); + Assert.assertEquals(6, schema.getTotalSegments()); + metadatas = schema + .getSegmentMetadataSnapshot() + .values() + .stream() + .filter(metadata -> datasource.equals(metadata.getSegment().getDataSource())) + .collect(Collectors.toList()); + Assert.assertEquals(0, metadatas.size()); + } } From df6d3dda699e293cec63d6b14daf10aea7d8f952 Mon Sep 17 00:00:00 2001 From: rishabh singh Date: Tue, 13 Aug 2024 21:12:49 +0530 Subject: [PATCH 4/6] Revert some changes --- .../loading/TombstoneSegmentizerFactory.java | 34 ++++--------------- .../apache/druid/client/BrokerServerView.java | 2 +- .../druid/client/CachingClusteredClient.java | 2 +- .../server/coordination/ServerManager.java | 8 ++--- 4 files changed, 13 insertions(+), 33 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java b/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java index e8d63ec899a2..72958e8049d3 100644 --- a/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/loading/TombstoneSegmentizerFactory.java @@ -30,17 +30,14 @@ import org.apache.druid.segment.SegmentLazyLoadFailCallback; import org.apache.druid.segment.StorageAdapter; import org.apache.druid.segment.column.ColumnHolder; -import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.data.Indexed; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; -import org.joda.time.DateTime; import org.joda.time.Interval; import javax.annotation.Nullable; + import java.io.File; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -77,13 +74,13 @@ public Interval getDataInterval() @Override public int getNumRows() { - return 0; + throw new UnsupportedOperationException(); } @Override public Indexed getAvailableDimensions() { - return Indexed.empty(); + throw new UnsupportedOperationException(); } @Override @@ -96,13 +93,13 @@ public BitmapFactory getBitmapFactoryForDimensions() @Override public Metadata getMetadata() { - return null; + throw new UnsupportedOperationException(); } @Override public Map getDimensionHandlers() { - return new HashMap<>(); + throw new UnsupportedOperationException(); } @Override @@ -114,7 +111,7 @@ public void close() @Override public List getColumnNames() { - return new ArrayList<>(); + throw new UnsupportedOperationException(); } @Nullable @@ -123,6 +120,7 @@ public ColumnHolder getColumnHolder(String columnName) { throw new UnsupportedOperationException(); } + }; final QueryableIndexStorageAdapter storageAdapter = new QueryableIndexStorageAdapter(queryableIndex) @@ -132,24 +130,6 @@ public boolean isFromTombstone() { return true; } - - @Override - public DateTime getMinTime() - { - return tombstone.getInterval().getStart(); - } - - @Override - public DateTime getMaxTime() - { - return tombstone.getInterval().getEnd(); - } - - @Override - public RowSignature getRowSignature() - { - return RowSignature.builder().build(); - } }; Segment segmentObject = new Segment() diff --git a/server/src/main/java/org/apache/druid/client/BrokerServerView.java b/server/src/main/java/org/apache/druid/client/BrokerServerView.java index cd95b4b73d6e..2cb2bec03b59 100644 --- a/server/src/main/java/org/apache/druid/client/BrokerServerView.java +++ b/server/src/main/java/org/apache/druid/client/BrokerServerView.java @@ -254,7 +254,7 @@ private void serverAddedSegment(final DruidServerMetadata server, final DataSegm VersionedIntervalTimeline timeline = timelines.get(segment.getDataSource()); if (timeline == null) { // broker needs to skip tombstones - timeline = new VersionedIntervalTimeline<>(Ordering.natural(), false); + timeline = new VersionedIntervalTimeline<>(Ordering.natural(), true); timelines.put(segment.getDataSource(), timeline); } diff --git a/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java b/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java index 01fd77b598c7..e4027bcd3574 100644 --- a/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java +++ b/server/src/main/java/org/apache/druid/client/CachingClusteredClient.java @@ -884,7 +884,7 @@ public TimelineLookup apply(TimelineLookup timeline) Objects::nonNull ); final VersionedIntervalTimeline newTimeline = - new VersionedIntervalTimeline<>(Ordering.natural(), false); + new VersionedIntervalTimeline<>(Ordering.natural(), true); // VersionedIntervalTimeline#addAll implementation is much more efficient than calling VersionedIntervalTimeline#add // in a loop when there are lot of segments to be added for same interval and version. newTimeline.addAll(iterator); diff --git a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java index 5674d35da8ed..b479737c9940 100644 --- a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java +++ b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java @@ -260,7 +260,6 @@ protected QueryRunner buildQueryRunnerForSegment( final ReferenceCountingSegment segment = chunk.getObject(); return buildAndDecorateQueryRunner( - query.getType(), factory, toolChest, segmentMapFn.apply(segment), @@ -271,7 +270,6 @@ protected QueryRunner buildQueryRunnerForSegment( } private QueryRunner buildAndDecorateQueryRunner( - final String queryType, final QueryRunnerFactory> factory, final QueryToolChest> toolChest, final SegmentReference segment, @@ -280,6 +278,9 @@ private QueryRunner buildAndDecorateQueryRunner( final AtomicLong cpuTimeAccumulator ) { + + + final SpecificSegmentSpec segmentSpec = new SpecificSegmentSpec(segmentDescriptor); final SegmentId segmentId = segment.getId(); final Interval segmentInterval = segment.getDataInterval(); @@ -293,10 +294,9 @@ private QueryRunner buildAndDecorateQueryRunner( StorageAdapter storageAdapter = segment.asStorageAdapter(); // Short-circuit when the index comes from a tombstone (it has no data by definition), // check for null also since no all segments (higher level ones) will have QueryableIndex... - if (storageAdapter.isFromTombstone() && queryType.equals(Query.SEGMENT_METADATA)) { + if (storageAdapter.isFromTombstone()) { return new NoopQueryRunner<>(); } - String segmentIdString = segmentId.toString(); MetricsEmittingQueryRunner metricsEmittingQueryRunnerInner = new MetricsEmittingQueryRunner<>( From 4bc77e4da97e32d4f8ee29f31770057ee9a20aaf Mon Sep 17 00:00:00 2001 From: rishabh singh Date: Tue, 13 Aug 2024 22:07:10 +0530 Subject: [PATCH 5/6] checkstyle --- .../segment/metadata/CoordinatorSegmentMetadataCacheTest.java | 2 -- .../sql/calcite/schema/BrokerSegmentMetadataCacheTest.java | 2 -- 2 files changed, 4 deletions(-) diff --git a/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java b/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java index 8744dbb07e66..772a79ae0ad1 100644 --- a/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java +++ b/server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java @@ -65,7 +65,6 @@ import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.incremental.IncrementalIndexSchema; -import org.apache.druid.segment.loading.LoadSpec; import org.apache.druid.segment.realtime.appenderator.SegmentSchemas; import org.apache.druid.segment.writeout.OffHeapMemorySegmentWriteOutMediumFactory; import org.apache.druid.server.QueryLifecycle; @@ -82,7 +81,6 @@ import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.LinearShardSpec; -import org.apache.druid.timeline.partition.NumberedShardSpec; import org.apache.druid.timeline.partition.TombstoneShardSpec; import org.easymock.EasyMock; import org.joda.time.Period; diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java index e22621b2bfb5..10504ca49d55 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/schema/BrokerSegmentMetadataCacheTest.java @@ -55,13 +55,11 @@ import org.apache.druid.segment.QueryableIndex; import org.apache.druid.segment.QueryableIndexStorageAdapter; import org.apache.druid.segment.TestHelper; -import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.incremental.IncrementalIndexSchema; import org.apache.druid.segment.metadata.AbstractSegmentMetadataCache; import org.apache.druid.segment.metadata.AvailableSegmentMetadata; import org.apache.druid.segment.metadata.CentralizedDatasourceSchemaConfig; -import org.apache.druid.segment.metadata.CoordinatorSegmentMetadataCache; import org.apache.druid.segment.metadata.DataSourceInformation; import org.apache.druid.segment.writeout.OffHeapMemorySegmentWriteOutMediumFactory; import org.apache.druid.server.QueryLifecycle; From 7342cc1b6a6074476c41251f4e489c318552ef89 Mon Sep 17 00:00:00 2001 From: rishabh singh Date: Tue, 13 Aug 2024 22:29:35 +0530 Subject: [PATCH 6/6] Update docs --- .../segment/metadata/AbstractSegmentMetadataCache.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java b/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java index fbe3a7c7839f..06ce009c8b2b 100644 --- a/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java +++ b/server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java @@ -458,10 +458,10 @@ public int getTotalSegments() @VisibleForTesting public void addSegment(final DruidServerMetadata server, final DataSegment segment) { - // Skip adding tombstone segment to the cache, as they do not have any data or column - // If they are added, the cache tries to refresh these segments indefinitely - // since segment metadata queries doesn't reflect metadata in tombstones. - // Refer: https://github.com/apache/druid/pull/12137 + // Skip adding tombstone segment to the cache. These segments lack data or column information. + // Additionally, segment metadata queries, which are not yet implemented for tombstone segments + // (see: https://github.com/apache/druid/pull/12137) do not provide metadata for tombstones, + // leading to indefinite refresh attempts for these segments. if (segment.isTombstone()) { return; }