From 8829ed7345cfc3df997df09f60219bb220364ae0 Mon Sep 17 00:00:00 2001 From: ken <1647023764@qq.com> Date: Sat, 25 May 2024 10:30:23 +0800 Subject: [PATCH] fix OrderedExecutor lost some metric (#4374) Fix [#4373](https://github.com/apache/bookkeeper/issues/4373) ### Motivation As shown in the issue. This is the first pr to fix the lost metric problem. And the other pr improve the metric in SingleThreadExecutor. Co-authored-by: fanjianye (cherry picked from commit e5300a0a2cbee6f50e20e24e79d1927d6500652a) --- .../common/util/OrderedExecutor.java | 46 ++------------- .../common/util/TestOrderedExecutor.java | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 41 deletions(-) create mode 100644 bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutor.java diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java index 982a5d49516..f856a4ea329 100644 --- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java +++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java @@ -391,6 +391,10 @@ protected OrderedExecutor(String baseName, int numThreads, ThreadFactory threadF ExecutorService thread = createSingleThreadExecutor( new ThreadFactoryBuilder().setNameFormat(name + "-" + getClass().getSimpleName() + "-" + i + "-%d") .setThreadFactory(threadFactory).build()); + SingleThreadExecutor ste = null; + if (thread instanceof SingleThreadExecutor) { + ste = (SingleThreadExecutor) thread; + } if (traceTaskExecution || preserveMdcForTaskExecution) { thread = addExecutorDecorators(thread); @@ -425,48 +429,8 @@ protected OrderedExecutor(String baseName, int numThreads, ThreadFactory threadF throw new RuntimeException("Couldn't start thread " + i, e); } - if (thread instanceof SingleThreadExecutor) { - SingleThreadExecutor ste = (SingleThreadExecutor) thread; + if (ste != null) { ste.registerMetrics(statsLogger); - } else if (thread instanceof ThreadPoolExecutor) { - ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) thread; - // Register gauges - statsLogger.scopeLabel("thread", String.valueOf(idx)) - .registerGauge(String.format("%s-queue", name), new Gauge() { - @Override - public Number getDefaultValue() { - return 0; - } - - @Override - public Number getSample() { - return threadPoolExecutor.getQueue().size(); - } - }); - statsLogger.scopeLabel("thread", String.valueOf(idx)) - .registerGauge(String.format("%s-completed-tasks", name), new Gauge() { - @Override - public Number getDefaultValue() { - return 0; - } - - @Override - public Number getSample() { - return threadPoolExecutor.getCompletedTaskCount(); - } - }); - statsLogger.scopeLabel("thread", String.valueOf(idx)) - .registerGauge(String.format("%s-total-tasks", name), new Gauge() { - @Override - public Number getDefaultValue() { - return 0; - } - - @Override - public Number getSample() { - return threadPoolExecutor.getTaskCount(); - } - }); } } diff --git a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutor.java b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutor.java new file mode 100644 index 00000000000..f8419ec3049 --- /dev/null +++ b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestOrderedExecutor.java @@ -0,0 +1,58 @@ +/* + * + * 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.bookkeeper.common.util; + +import org.apache.bookkeeper.test.TestStatsProvider; +import org.junit.Assert; +import org.junit.Test; + +/** + * Test OrderedExecutor/Scheduler . + */ +public class TestOrderedExecutor { + + @Test + public void testOrderExecutorPrometheusMetric() { + testGenerateMetric(false); + testGenerateMetric(true); + } + + private void testGenerateMetric(boolean isTraceTaskExecution) { + TestStatsProvider provider = new TestStatsProvider(); + + TestStatsProvider.TestStatsLogger rootStatsLogger = provider.getStatsLogger(""); + TestStatsProvider.TestStatsLogger bookieStats = + (TestStatsProvider.TestStatsLogger) rootStatsLogger.scope("bookkeeper_server"); + + OrderedExecutor executor = OrderedExecutor.newBuilder().statsLogger(bookieStats) + .name("test").numThreads(1).traceTaskExecution(isTraceTaskExecution).build(); + + TestStatsProvider.TestStatsLogger testStatsLogger = (TestStatsProvider.TestStatsLogger) + bookieStats.scope("thread_test_OrderedExecutor_0_0"); + + Assert.assertNotNull(testStatsLogger.getGauge("thread_executor_queue").getSample()); + Assert.assertNotNull(testStatsLogger.getGauge("thread_executor_completed").getSample()); + Assert.assertNotNull(testStatsLogger.getGauge("thread_executor_tasks_completed").getSample()); + Assert.assertNotNull(testStatsLogger.getGauge("thread_executor_tasks_rejected").getSample()); + Assert.assertNotNull(testStatsLogger.getGauge("thread_executor_tasks_failed").getSample()); + } +}