This is an automated email from the ASF dual-hosted git repository.
abhishek 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 5772dfd155 Peons should not report SysMonitor stats since
MiddleManager reports them. (#12802)
5772dfd155 is described below
commit 5772dfd1552ab18276a7f8de9335e3a1ef489733
Author: Tejaswini Bandlamudi <[email protected]>
AuthorDate: Sat Jul 23 13:32:16 2022 +0530
Peons should not report SysMonitor stats since MiddleManager reports them.
(#12802)
Sysmonitor stats (mem, fs, disk, net, cpu, swap, sys, tcp) are reported by
all Druid processes, including Peons that are ephemeral in nature. Since Peons
always run on the same host as the MiddleManager that spawned them and is
unlikely to change, the SyMonitor metrics emitted by Peon are merely
duplicates. This is often not a problem except when machines are super-beefy.
Imagine a 64-core machine and 32 workers running on this machine. now you will
have each Peon reporting metrics fo [...]
This PR updates MetricsModule to check node role running while registering
SysMonitor and not to load any existing SysMonitor$Stats.
---
.../druid/java/util/metrics/NoopSysMonitor.java | 36 +++++++++++++
.../java/util/metrics/NoopSysMonitorTest.java | 42 +++++++++++++++
.../apache/druid/server/metrics/MetricsModule.java | 50 +++++++++++++++---
.../druid/server/metrics/MetricsModuleTest.java | 61 ++++++++++++++++++++++
4 files changed, 183 insertions(+), 6 deletions(-)
diff --git
a/core/src/main/java/org/apache/druid/java/util/metrics/NoopSysMonitor.java
b/core/src/main/java/org/apache/druid/java/util/metrics/NoopSysMonitor.java
new file mode 100644
index 0000000000..82ee742db9
--- /dev/null
+++ b/core/src/main/java/org/apache/druid/java/util/metrics/NoopSysMonitor.java
@@ -0,0 +1,36 @@
+/*
+ * 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.java.util.metrics;
+
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+
+public class NoopSysMonitor extends SysMonitor
+{
+ public NoopSysMonitor()
+ {
+ super();
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ return false;
+ }
+}
diff --git
a/core/src/test/java/org/apache/druid/java/util/metrics/NoopSysMonitorTest.java
b/core/src/test/java/org/apache/druid/java/util/metrics/NoopSysMonitorTest.java
new file mode 100644
index 0000000000..12a7f1b028
--- /dev/null
+++
b/core/src/test/java/org/apache/druid/java/util/metrics/NoopSysMonitorTest.java
@@ -0,0 +1,42 @@
+/*
+ * 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.java.util.metrics;
+
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class NoopSysMonitorTest
+{
+ private static final String CPU_ARCH = System.getProperty("os.arch");
+
+ @Test
+ public void testDoMonitor()
+ {
+ Assume.assumeFalse("aarch64".equals(CPU_ARCH));
+
+ ServiceEmitter serviceEmitter = Mockito.mock(ServiceEmitter.class);
+ NoopSysMonitor noopSysMonitor = new NoopSysMonitor();
+
+ Assert.assertFalse(noopSysMonitor.doMonitor(serviceEmitter));
+ }
+}
diff --git
a/server/src/main/java/org/apache/druid/server/metrics/MetricsModule.java
b/server/src/main/java/org/apache/druid/server/metrics/MetricsModule.java
index df6fe8bc8d..b4a0df6cd9 100644
--- a/server/src/main/java/org/apache/druid/server/metrics/MetricsModule.java
+++ b/server/src/main/java/org/apache/druid/server/metrics/MetricsModule.java
@@ -26,12 +26,15 @@ import com.google.inject.Injector;
import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.Provides;
+import com.google.inject.TypeLiteral;
import com.google.inject.name.Names;
import io.timeandspace.cronscheduler.CronScheduler;
+import org.apache.druid.discovery.NodeRole;
import org.apache.druid.guice.DruidBinders;
import org.apache.druid.guice.JsonConfigProvider;
import org.apache.druid.guice.LazySingleton;
import org.apache.druid.guice.ManageLifecycle;
+import org.apache.druid.guice.annotations.Self;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.concurrent.Execs;
import org.apache.druid.java.util.common.logger.Logger;
@@ -43,9 +46,11 @@ import org.apache.druid.java.util.metrics.JvmMonitor;
import org.apache.druid.java.util.metrics.JvmThreadsMonitor;
import org.apache.druid.java.util.metrics.Monitor;
import org.apache.druid.java.util.metrics.MonitorScheduler;
+import org.apache.druid.java.util.metrics.NoopSysMonitor;
import org.apache.druid.java.util.metrics.SysMonitor;
import org.apache.druid.query.ExecutorServiceMonitor;
+import javax.annotation.Nullable;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
@@ -170,13 +175,46 @@ public class MetricsModule implements Module
@Provides
@ManageLifecycle
public SysMonitor getSysMonitor(
- DataSourceTaskIdHolder dataSourceTaskIdHolder
+ DataSourceTaskIdHolder dataSourceTaskIdHolder,
+ Injector injector
)
{
- Map<String, String[]> dimensions = MonitorsConfig.mapOfDatasourceAndTaskID(
- dataSourceTaskIdHolder.getDataSource(),
- dataSourceTaskIdHolder.getTaskId()
- );
- return new SysMonitor(dimensions);
+ final Set<NodeRole> nodeRoles = getNodeRoles(injector);
+
+ if (isPeonRole(nodeRoles)) {
+ return new NoopSysMonitor();
+ } else {
+ Map<String, String[]> dimensions =
MonitorsConfig.mapOfDatasourceAndTaskID(
+ dataSourceTaskIdHolder.getDataSource(),
+ dataSourceTaskIdHolder.getTaskId()
+ );
+ return new SysMonitor(dimensions);
+ }
+ }
+
+ @Nullable
+ private static Set<NodeRole> getNodeRoles(Injector injector)
+ {
+ try {
+ return injector.getInstance(
+ Key.get(
+ new TypeLiteral<Set<NodeRole>>()
+ {
+ },
+ Self.class
+ )
+ );
+ }
+ catch (Exception e) {
+ return null;
+ }
+ }
+
+ private static boolean isPeonRole(Set<NodeRole> nodeRoles)
+ {
+ if (nodeRoles == null) {
+ return false;
+ }
+ return nodeRoles.contains(NodeRole.PEON);
}
}
diff --git
a/server/src/test/java/org/apache/druid/server/metrics/MetricsModuleTest.java
b/server/src/test/java/org/apache/druid/server/metrics/MetricsModuleTest.java
index cba4f84dbd..26d1ecb54a 100644
---
a/server/src/test/java/org/apache/druid/server/metrics/MetricsModuleTest.java
+++
b/server/src/test/java/org/apache/druid/server/metrics/MetricsModuleTest.java
@@ -20,6 +20,7 @@
package org.apache.druid.server.metrics;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.inject.Binder;
import com.google.inject.CreationException;
import com.google.inject.Guice;
@@ -27,7 +28,9 @@ import com.google.inject.Injector;
import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.Scopes;
+import com.google.inject.TypeLiteral;
import com.google.inject.name.Names;
+import org.apache.druid.discovery.NodeRole;
import org.apache.druid.guice.GuiceInjectors;
import org.apache.druid.guice.JsonConfigProvider;
import org.apache.druid.guice.LazySingleton;
@@ -37,22 +40,31 @@ import org.apache.druid.initialization.Initialization;
import org.apache.druid.jackson.JacksonModule;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.emitter.service.ServiceEventBuilder;
import org.apache.druid.java.util.metrics.BasicMonitorScheduler;
import org.apache.druid.java.util.metrics.ClockDriftSafeMonitorScheduler;
import org.apache.druid.java.util.metrics.MonitorScheduler;
+import org.apache.druid.java.util.metrics.NoopSysMonitor;
+import org.apache.druid.java.util.metrics.SysMonitor;
import org.apache.druid.server.DruidNode;
import org.hamcrest.CoreMatchers;
import org.junit.Assert;
+import org.junit.Assume;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mockito;
import javax.validation.Validation;
import javax.validation.Validator;
import java.util.Properties;
+import java.util.Set;
public class MetricsModuleTest
{
+ private static final String CPU_ARCH = System.getProperty("os.arch");
+
@Rule
public ExpectedException expectedException = ExpectedException.none();
@@ -155,6 +167,55 @@ public class MetricsModuleTest
createInjector(properties).getInstance(MonitorScheduler.class);
}
+ @Test
+ public void testGetSysMonitorViaInjector()
+ {
+ // Do not run the tests on ARM64. Sigar library has no binaries for ARM64
+ Assume.assumeFalse("aarch64".equals(CPU_ARCH));
+
+ final NodeRole nodeRole = NodeRole.PEON;
+ final Injector injector = Guice.createInjector(
+ new JacksonModule(),
+ new LifecycleModule(),
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ },
+ binder -> {
+ binder.bind(
+ new TypeLiteral<Set<NodeRole>>()
+ {
+
}).annotatedWith(Self.class).toInstance(ImmutableSet.of(nodeRole));
+ }
+ );
+ final DataSourceTaskIdHolder dimensionIdHolder = new
DataSourceTaskIdHolder();
+ injector.injectMembers(dimensionIdHolder);
+ final MetricsModule metricsModule = new MetricsModule();
+ final SysMonitor sysMonitor =
metricsModule.getSysMonitor(dimensionIdHolder, injector);
+ final ServiceEmitter emitter = Mockito.mock(ServiceEmitter.class);
+ sysMonitor.doMonitor(emitter);
+
+ Assert.assertTrue(sysMonitor instanceof NoopSysMonitor);
+ Mockito.verify(emitter,
Mockito.never()).emit(ArgumentMatchers.any(ServiceEventBuilder.class));
+ }
+
+ @Test
+ public void testGetSysMonitorWhenNull()
+ {
+ // Do not run the tests on ARM64. Sigar library has no binaries for ARM64
+ Assume.assumeFalse("aarch64".equals(CPU_ARCH));
+
+ final Injector injector = createInjector(new Properties());
+ final DataSourceTaskIdHolder dimensionIdHolder = new
DataSourceTaskIdHolder();
+ injector.injectMembers(dimensionIdHolder);
+ final MetricsModule metricsModule = new MetricsModule();
+ final SysMonitor sysMonitor =
metricsModule.getSysMonitor(dimensionIdHolder, injector);
+ final ServiceEmitter emitter = Mockito.mock(ServiceEmitter.class);
+ sysMonitor.doMonitor(emitter);
+
+ Assert.assertFalse(sysMonitor instanceof NoopSysMonitor);
+ Mockito.verify(emitter,
Mockito.atLeastOnce()).emit(ArgumentMatchers.any(ServiceEventBuilder.class));
+ }
+
private static Injector createInjector(Properties properties)
{
return Guice.createInjector(
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
