This is an automated email from the ASF dual-hosted git repository.
kfaraz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 142f76cf821 Fix binding of segment metadata cache on CliOverlord
(#17772)
142f76cf821 is described below
commit 142f76cf821788b84083354d9ebe6265fe06cb49
Author: Kashif Faraz <[email protected]>
AuthorDate: Mon Mar 3 17:28:16 2025 +0530
Fix binding of segment metadata cache on CliOverlord (#17772)
Changes
---------
- Bind `SegmentMetadataCache` only once to
`HeapMemorySegmentMetadataCache` in `SQLMetadataStorageDruidModule`
- Invoke start and stop of the cache from `DruidOverlord` rather than on
lifecycle start/stop
- Do not override the binding in `CliOverlord`
---
.../mysql/MySQLMetadataStorageModuleTest.java | 8 ++++
.../druid/indexing/overlord/DruidOverlord.java | 4 ++
.../druid/guice/SQLMetadataStorageDruidModule.java | 8 ++--
.../cache/HeapMemorySegmentMetadataCache.java | 4 --
.../java/org/apache/druid/cli/CliOverlord.java | 7 ----
.../java/org/apache/druid/cli/CliOverlordTest.java | 44 ++++++++++++++++++++++
6 files changed, 60 insertions(+), 15 deletions(-)
diff --git
a/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLMetadataStorageModuleTest.java
b/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLMetadataStorageModuleTest.java
index fee116bfa3a..cbc3e3ff65e 100644
---
a/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLMetadataStorageModuleTest.java
+++
b/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLMetadataStorageModuleTest.java
@@ -35,6 +35,7 @@ import org.apache.druid.guice.LifecycleModule;
import org.apache.druid.guice.MetadataConfigModule;
import org.apache.druid.guice.annotations.Json;
import org.apache.druid.guice.security.EscalatorModule;
+import org.apache.druid.java.util.common.concurrent.ScheduledExecutorFactory;
import org.apache.druid.java.util.emitter.core.NoopEmitter;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
import org.junit.Assert;
@@ -139,6 +140,13 @@ public class MySQLMetadataStorageModuleTest
// A provider for DruidLeaderSelector is needed by
SqlSegmentMetadataTransactionFactory
return null;
}
+
+ @Provides
+ public ScheduledExecutorFactory getExecutorFactory()
+ {
+ // Required for HeapMemorySegmentMetadataCache
+ return null;
+ }
}
)
);
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/DruidOverlord.java
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/DruidOverlord.java
index f03b2fd48b8..9aacf6dea84 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/DruidOverlord.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/DruidOverlord.java
@@ -59,6 +59,7 @@ public class DruidOverlord
private final DruidLeaderSelector overlordLeaderSelector;
private final DruidLeaderSelector.Listener leadershipListener;
+ private final SegmentMetadataCache segmentMetadataCache;
private final ReentrantLock giant = new ReentrantLock(true);
@@ -98,6 +99,7 @@ public class DruidOverlord
)
{
this.overlordLeaderSelector = overlordLeaderSelector;
+ this.segmentMetadataCache = segmentMetadataCache;
final DruidNode node =
coordinatorOverlordServiceConfig.getOverlordService() == null ? selfNode :
selfNode.withService(coordinatorOverlordServiceConfig.getOverlordService());
@@ -225,6 +227,7 @@ public class DruidOverlord
giant.lock();
try {
+ segmentMetadataCache.start();
overlordLeaderSelector.registerListener(leadershipListener);
}
finally {
@@ -244,6 +247,7 @@ public class DruidOverlord
try {
gracefulStopLeaderLifecycle();
overlordLeaderSelector.unregisterListener();
+ segmentMetadataCache.stop();
}
finally {
giant.unlock();
diff --git
a/server/src/main/java/org/apache/druid/guice/SQLMetadataStorageDruidModule.java
b/server/src/main/java/org/apache/druid/guice/SQLMetadataStorageDruidModule.java
index e5dabdb8fe4..7cb48c694ef 100644
---
a/server/src/main/java/org/apache/druid/guice/SQLMetadataStorageDruidModule.java
+++
b/server/src/main/java/org/apache/druid/guice/SQLMetadataStorageDruidModule.java
@@ -41,7 +41,7 @@ import org.apache.druid.metadata.SqlSegmentsMetadataManager;
import org.apache.druid.metadata.SqlSegmentsMetadataManagerProvider;
import org.apache.druid.metadata.segment.SegmentMetadataTransactionFactory;
import org.apache.druid.metadata.segment.SqlSegmentMetadataTransactionFactory;
-import org.apache.druid.metadata.segment.cache.NoopSegmentMetadataCache;
+import org.apache.druid.metadata.segment.cache.HeapMemorySegmentMetadataCache;
import org.apache.druid.metadata.segment.cache.SegmentMetadataCache;
import org.apache.druid.server.audit.AuditManagerConfig;
import org.apache.druid.server.audit.AuditSerdeHelper;
@@ -105,10 +105,10 @@ public class SQLMetadataStorageDruidModule implements
Module
.to(SQLMetadataRuleManagerProvider.class)
.in(LazySingleton.class);
- // SegmentMetadataCache is used only by the Overlord
- // Bind to noop implementation here to fulfill dependencies
+ // SegmentMetadataCache is bound for all services but is used only by the
Overlord
+ // Similar to some other classes bound here, such as
IndexerSQLMetadataStorageCoordinator
binder.bind(SegmentMetadataCache.class)
- .to(NoopSegmentMetadataCache.class)
+ .to(HeapMemorySegmentMetadataCache.class)
.in(LazySingleton.class);
PolyBind.optionBinder(binder,
Key.get(SegmentMetadataTransactionFactory.class))
diff --git
a/server/src/main/java/org/apache/druid/metadata/segment/cache/HeapMemorySegmentMetadataCache.java
b/server/src/main/java/org/apache/druid/metadata/segment/cache/HeapMemorySegmentMetadataCache.java
index 4a451d162e6..65482e41046 100644
---
a/server/src/main/java/org/apache/druid/metadata/segment/cache/HeapMemorySegmentMetadataCache.java
+++
b/server/src/main/java/org/apache/druid/metadata/segment/cache/HeapMemorySegmentMetadataCache.java
@@ -35,8 +35,6 @@ import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.Stopwatch;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.concurrent.ScheduledExecutorFactory;
-import org.apache.druid.java.util.common.lifecycle.LifecycleStart;
-import org.apache.druid.java.util.common.lifecycle.LifecycleStop;
import org.apache.druid.java.util.common.parsers.CloseableIterator;
import org.apache.druid.java.util.emitter.EmittingLogger;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
@@ -135,7 +133,6 @@ public class HeapMemorySegmentMetadataCache implements
SegmentMetadataCache
@Override
- @LifecycleStart
public void start()
{
if (!isCacheEnabled) {
@@ -152,7 +149,6 @@ public class HeapMemorySegmentMetadataCache implements
SegmentMetadataCache
}
@Override
- @LifecycleStop
public void stop()
{
synchronized (cacheStateLock) {
diff --git a/services/src/main/java/org/apache/druid/cli/CliOverlord.java
b/services/src/main/java/org/apache/druid/cli/CliOverlord.java
index 7abe1ec55ca..e73d87d8bfe 100644
--- a/services/src/main/java/org/apache/druid/cli/CliOverlord.java
+++ b/services/src/main/java/org/apache/druid/cli/CliOverlord.java
@@ -111,8 +111,6 @@ import org.apache.druid.java.util.common.logger.Logger;
import org.apache.druid.metadata.SegmentsMetadataManager;
import org.apache.druid.metadata.SegmentsMetadataManagerProvider;
import org.apache.druid.metadata.input.InputSourceModule;
-import org.apache.druid.metadata.segment.cache.HeapMemorySegmentMetadataCache;
-import org.apache.druid.metadata.segment.cache.SegmentMetadataCache;
import org.apache.druid.query.lookup.LookupSerdeModule;
import org.apache.druid.segment.incremental.RowIngestionMetersFactory;
import org.apache.druid.segment.metadata.CentralizedDatasourceSchemaConfig;
@@ -231,11 +229,6 @@ public class CliOverlord extends ServerRunnable
JsonConfigProvider.bind(binder, "druid.indexer.task.default",
DefaultTaskConfig.class);
binder.bind(RetryPolicyFactory.class).in(LazySingleton.class);
- // Override the default binding of SegmentMetadataCache done in
SQLMetadataStorageDruidModule
- binder.bind(SegmentMetadataCache.class)
- .to(HeapMemorySegmentMetadataCache.class)
- .in(ManageLifecycle.class);
-
binder.bind(DruidOverlord.class).in(ManageLifecycle.class);
binder.bind(TaskMaster.class).in(ManageLifecycle.class);
binder.bind(TaskCountStatsProvider.class).to(TaskMaster.class);
diff --git a/services/src/test/java/org/apache/druid/cli/CliOverlordTest.java
b/services/src/test/java/org/apache/druid/cli/CliOverlordTest.java
new file mode 100644
index 00000000000..23efafe89f8
--- /dev/null
+++ b/services/src/test/java/org/apache/druid/cli/CliOverlordTest.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.cli;
+
+import com.google.inject.Injector;
+import org.apache.druid.guice.StartupInjectorBuilder;
+import org.apache.druid.metadata.segment.cache.HeapMemorySegmentMetadataCache;
+import org.apache.druid.metadata.segment.cache.SegmentMetadataCache;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class CliOverlordTest
+{
+ @Test
+ public void testSegmentMetadataCacheIsBound()
+ {
+ final Injector injector = new StartupInjectorBuilder().forServer().build();
+ final CliOverlord overlord = new CliOverlord();
+ injector.injectMembers(overlord);
+
+ final Injector overlordInjector = overlord.makeInjector();
+
+ final SegmentMetadataCache segmentMetadataCache
+ = overlordInjector.getInstance(SegmentMetadataCache.class);
+ Assert.assertTrue(segmentMetadataCache instanceof
HeapMemorySegmentMetadataCache);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
