You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "abstractdog (via GitHub)" <gi...@apache.org> on 2024/11/24 15:45:26 UTC
[PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
abstractdog opened a new pull request, #5557:
URL: https://github.com/apache/hive/pull/5557
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### Is the change a dependency upgrade?
<!--
If yes, please attach a file with output from mvn dependency:tree to validate a complete upgrade of dependency.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856247776
##########
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/StatsRecordingThreadPool.java:
##########
@@ -161,98 +149,44 @@ private void setupMDCFromNDC(final Callable<V> actualCallable) {
}
}
- /**
- * LLAP IO related counters.
- */
- public enum LlapExecutorCounters {
- EXECUTOR_CPU_NS,
- EXECUTOR_USER_NS;
-
- }
-
- private void updateCounters(final List<LlapUtil.StatisticsData> statsBefore,
- final Callable<V> actualCallable, long cpuTime, long userTime) {
+ private void updateCounters(final LlapThreadLocalStatistics statsBefore,
+ final Callable<V> actualCallable) {
Thread thread = Thread.currentThread();
- TezCounters tezCounters = null;
- // add tez counters for task execution and llap io
- if (actualCallable instanceof TaskRunner2Callable) {
- TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
- // counters for task execution side
- tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
- } else if (actualCallable instanceof TezCounterSource) {
- // Other counter sources (currently used in LLAP IO).
- tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
- } else {
- LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
- }
+ final TezCounters tezCounters = getTezCounters(actualCallable);
if (tezCounters != null) {
- if (cpuTime >= 0 && userTime >= 0) {
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
- }
if (statsBefore != null) {
- // if there are multiple stats for the same scheme (from different NameNode), this
- // method will squash them together
- Map<String, FileSystem.Statistics> schemeToStats = LlapUtil
- .getCombinedFileSystemStatistics();
- for (Map.Entry<String, FileSystem.Statistics> entry : schemeToStats.entrySet()) {
- final String scheme = entry.getKey();
- FileSystem.Statistics statistics = entry.getValue();
- FileSystem.Statistics.StatisticsData threadFSStats = statistics
- .getThreadStatistics();
- List<LlapUtil.StatisticsData> allStatsBefore = LlapUtil
- .getStatisticsForScheme(scheme, statsBefore);
- long bytesReadDelta = 0;
- long bytesWrittenDelta = 0;
- long readOpsDelta = 0;
- long largeReadOpsDelta = 0;
- long writeOpsDelta = 0;
- // there could be more scheme after execution as execution might be accessing a
- // different filesystem. So if we don't find a matching scheme before execution we
- // just use the after execution values directly without computing delta difference
- if (allStatsBefore != null && !allStatsBefore.isEmpty()) {
- for (LlapUtil.StatisticsData sb : allStatsBefore) {
- bytesReadDelta += threadFSStats.getBytesRead() - sb.getBytesRead();
- bytesWrittenDelta += threadFSStats.getBytesWritten() - sb.getBytesWritten();
- readOpsDelta += threadFSStats.getReadOps() - sb.getReadOps();
- largeReadOpsDelta += threadFSStats.getLargeReadOps() - sb.getLargeReadOps();
- writeOpsDelta += threadFSStats.getWriteOps() - sb.getWriteOps();
- }
- } else {
- bytesReadDelta = threadFSStats.getBytesRead();
- bytesWrittenDelta = threadFSStats.getBytesWritten();
- readOpsDelta = threadFSStats.getReadOps();
- largeReadOpsDelta = threadFSStats.getLargeReadOps();
- writeOpsDelta = threadFSStats.getWriteOps();
- }
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ)
- .increment(bytesReadDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN)
- .increment(bytesWrittenDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(readOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS)
- .increment(largeReadOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS)
- .increment(writeOpsDelta);
+ LlapThreadLocalStatistics currentStats = new LlapThreadLocalStatistics(mxBean);
+ currentStats.subtract(statsBefore).fill(tezCounters);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} scheme: {} " +
- "bytesRead: {} bytesWritten: {} readOps: {} largeReadOps: {} writeOps: {}",
- actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(),
- scheme, bytesReadDelta, bytesWrittenDelta, readOpsDelta, largeReadOpsDelta,
- writeOpsDelta);
- }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} stats: {}",
+ actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(), currentStats);
}
} else {
LOG.warn("File system statistics snapshot before execution of thread is null." +
- "Thread name: {} id: {} allStats: {}", thread.getName(), thread.getId(),
- statsBefore);
+ "Thread name: {} id: {}", thread.getName(), thread.getId());
}
} else {
LOG.warn("TezCounters is null for callable type: {}",
actualCallable.getClass().getSimpleName());
}
}
}
+
+ private static <V> TezCounters getTezCounters(Callable<V> actualCallable) {
+ TezCounters tezCounters = null;
+ // add tez counters for task execution and llap io
+ if (actualCallable instanceof TaskRunner2Callable) {
+ TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
+ // counters for task execution side
+ tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
+ } else if (actualCallable instanceof TezCounterSource) {
+ // Other counter sources (currently used in LLAP IO).
+ tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
+ } else {
+ LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
Review Comment:
no, even if this is not expected, it should not cause the task to fail
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856249326
##########
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/StatsRecordingThreadPool.java:
##########
@@ -161,98 +149,44 @@ private void setupMDCFromNDC(final Callable<V> actualCallable) {
}
}
- /**
- * LLAP IO related counters.
- */
- public enum LlapExecutorCounters {
- EXECUTOR_CPU_NS,
- EXECUTOR_USER_NS;
-
- }
-
- private void updateCounters(final List<LlapUtil.StatisticsData> statsBefore,
- final Callable<V> actualCallable, long cpuTime, long userTime) {
+ private void updateCounters(final LlapThreadLocalStatistics statsBefore,
+ final Callable<V> actualCallable) {
Thread thread = Thread.currentThread();
- TezCounters tezCounters = null;
- // add tez counters for task execution and llap io
- if (actualCallable instanceof TaskRunner2Callable) {
- TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
- // counters for task execution side
- tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
- } else if (actualCallable instanceof TezCounterSource) {
- // Other counter sources (currently used in LLAP IO).
- tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
- } else {
- LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
- }
+ final TezCounters tezCounters = getTezCounters(actualCallable);
if (tezCounters != null) {
- if (cpuTime >= 0 && userTime >= 0) {
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
- }
if (statsBefore != null) {
- // if there are multiple stats for the same scheme (from different NameNode), this
- // method will squash them together
- Map<String, FileSystem.Statistics> schemeToStats = LlapUtil
- .getCombinedFileSystemStatistics();
- for (Map.Entry<String, FileSystem.Statistics> entry : schemeToStats.entrySet()) {
- final String scheme = entry.getKey();
- FileSystem.Statistics statistics = entry.getValue();
- FileSystem.Statistics.StatisticsData threadFSStats = statistics
- .getThreadStatistics();
- List<LlapUtil.StatisticsData> allStatsBefore = LlapUtil
- .getStatisticsForScheme(scheme, statsBefore);
- long bytesReadDelta = 0;
- long bytesWrittenDelta = 0;
- long readOpsDelta = 0;
- long largeReadOpsDelta = 0;
- long writeOpsDelta = 0;
- // there could be more scheme after execution as execution might be accessing a
- // different filesystem. So if we don't find a matching scheme before execution we
- // just use the after execution values directly without computing delta difference
- if (allStatsBefore != null && !allStatsBefore.isEmpty()) {
- for (LlapUtil.StatisticsData sb : allStatsBefore) {
- bytesReadDelta += threadFSStats.getBytesRead() - sb.getBytesRead();
- bytesWrittenDelta += threadFSStats.getBytesWritten() - sb.getBytesWritten();
- readOpsDelta += threadFSStats.getReadOps() - sb.getReadOps();
- largeReadOpsDelta += threadFSStats.getLargeReadOps() - sb.getLargeReadOps();
- writeOpsDelta += threadFSStats.getWriteOps() - sb.getWriteOps();
- }
- } else {
- bytesReadDelta = threadFSStats.getBytesRead();
- bytesWrittenDelta = threadFSStats.getBytesWritten();
- readOpsDelta = threadFSStats.getReadOps();
- largeReadOpsDelta = threadFSStats.getLargeReadOps();
- writeOpsDelta = threadFSStats.getWriteOps();
- }
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ)
- .increment(bytesReadDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN)
- .increment(bytesWrittenDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(readOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS)
- .increment(largeReadOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS)
- .increment(writeOpsDelta);
+ LlapThreadLocalStatistics currentStats = new LlapThreadLocalStatistics(mxBean);
+ currentStats.subtract(statsBefore).fill(tezCounters);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} scheme: {} " +
- "bytesRead: {} bytesWritten: {} readOps: {} largeReadOps: {} writeOps: {}",
- actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(),
- scheme, bytesReadDelta, bytesWrittenDelta, readOpsDelta, largeReadOpsDelta,
- writeOpsDelta);
- }
+ if (LOG.isDebugEnabled()) {
Review Comment:
this is the usual debug check in cases the string creation might be already expensive, so we don't even want to to it on INFO level
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "zratkai (via GitHub)" <gi...@apache.org>.
zratkai commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856322962
##########
llap-common/src/java/org/apache/hadoop/hive/llap/LlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.tez.common.counters.FileSystemCounter;
+import org.apache.tez.common.counters.TezCounters;
+import java.lang.management.ThreadMXBean;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+
+/**
+ * Convenience class for handling thread local statistics for different schemes in LLAP.
+ * The motivation is that thread local stats (available from FileSystem.getAllStatistics()) are in a List, which
+ * doesn't guarantee that for a single scheme, a single Statistics object is returned (e.g. in case of multiple
+ * namenodes). This class encapsulates that data, and takes care of merging them transparently.
+ * LlapThreadLocalStatistics is used in LLAP's task ThreadPool, where the current thread's statistics is calculated by a
+ * simple delta computation (before/after running the task callable), like:
+ *
+ * LlapThreadLocalStatistics statsBefore = new LlapThreadLocalStatistics(...);
+ * LlapThreadLocalStatistics diff = new LlapThreadLocalStatistics(...).subtract(statsBefore);
+ */
+public class LlapThreadLocalStatistics {
+
+ /**
+ * LLAP IO related counters.
+ */
+ public enum LlapExecutorCounters {
+ EXECUTOR_CPU_NS,
+ EXECUTOR_USER_NS;
+ }
+
+ @VisibleForTesting
+ Map<String, LlapThreadLocalStatistics.StatisticsData> schemeToThreadLocalStats = new HashMap<>();
+ @VisibleForTesting
+ long cpuTime;
+ @VisibleForTesting
+ long userTime;
+
+ /**
+ * In this constructor we create a snapshot of the current thread local statistics and take care of merging the
+ * ones that belong to the same scheme.
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean) {
+ this(mxBean, FileSystem.getAllStatistics());
+ }
+
+ /**
+ * Merges the list to a map.
+ * Input list:
+ * 1. FileSystem.Statistics (scheme: file)
+ * 2. FileSystem.Statistics (scheme: hdfs)
+ * 3. FileSystem.Statistics (scheme: hdfs)
+ * Output map:
+ * file : LlapThreadLocalStatistics.StatisticsData (1)
+ * hdfs : LlapThreadLocalStatistics.StatisticsData (2 + 3)
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean, List<FileSystem.Statistics> allStatistics) {
+ cpuTime = mxBean == null ? -1 : mxBean.getCurrentThreadCpuTime();
+ userTime = mxBean == null ? -1 : mxBean.getCurrentThreadUserTime();
+
+ for (FileSystem.Statistics statistics : allStatistics) {
+ schemeToThreadLocalStats.merge(statistics.getScheme(),
+ new StatisticsData(statistics.getThreadStatistics()),
+ (statsCurrent, statsNew) -> statsCurrent.merge(statistics.getThreadStatistics()));
+ }
+ }
+
+ // This method iterates on the other LlapThreadLocalStatistics's schemes, and subtract them from this one if it's
+ // present here too.
+ public LlapThreadLocalStatistics subtract(LlapThreadLocalStatistics other) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> otherStats :
+ other.schemeToThreadLocalStats.entrySet()){
+ schemeToThreadLocalStats.computeIfPresent(otherStats.getKey(),
+ (thisScheme, stats) -> stats.subtract(otherStats.getValue()));
+ }
+
+ cpuTime -= other.cpuTime;
+ userTime -= other.userTime;
+
+ return this;
+ }
+
+ public void fill(TezCounters tezCounters) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> threadLocalStats :
+ schemeToThreadLocalStats.entrySet()){
+ String scheme = threadLocalStats.getKey();
+ StatisticsData stats = threadLocalStats.getValue();
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ).increment(stats.bytesRead);
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN).increment(stats.bytesWritten);
+ tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(stats.readOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS).increment(stats.largeReadOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS).increment(stats.writeOps);
+ }
+
+ if (cpuTime >= 0 && userTime >= 0) {
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
+ }
+ }
+
+ public String toString(){
+ return String.format("LlapThreadLocalStatistics: %s", schemeToThreadLocalStats.toString());
+ }
+
+ /**
+ * Convenience class over Hadoop's FileSystem.Statistics.StatisticsData.
+ * Unfortunately, neither the fields, nor the convenience methods (e.g. StatisticsData.add, StatisticsData.negate)
+ * are available here as they are package protected, so we cannot reuse them.
+ */
+ public static class StatisticsData {
Review Comment:
Your choice. ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1870855491
##########
llap-common/src/test/org/apache/hadoop/hive/llap/TestLlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.fs.FileSystem;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.lang.management.ThreadMXBean;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+public class TestLlapThreadLocalStatistics {
+
+ private static final ThreadMXBean mxBean = LlapUtil.initThreadMxBean();
+
+ @Test
+ public void testEmptyStatistics() {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertEquals(0, after.subtract(before).schemeToThreadLocalStats.keySet().size());
+ }
+
+ @Test
+ public void testCpuTimeUserTime() throws Exception {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should be >0", before.cpuTime > 0);
+ Assert.assertTrue("userTime should be >0", before.userTime > 0);
+
+ Thread.sleep(100);
+
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should increase", after.cpuTime > before.cpuTime);
+ // userTime assertion is flaky, not checked here
+ }
+
+ @Test
+ public void testCountersMergedForTheSameScheme() {
+ LlapThreadLocalStatistics stats = new LlapThreadLocalStatistics(mxBean,
+ createMockStatistics(new String[]{"file", "hdfs", "hdfs"}, new Integer[]{1, 1, 1}));
Review Comment:
I don't think so, I cannot recall if we refactor file system schemes to constants in tests, IMO it just leads to more code with no value
UPDATE: actually, I have no strong opinions about this, let me refactor to:
```
private static final String FILE = "file";
private static final String HDFS = "hdfs";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1870855491
##########
llap-common/src/test/org/apache/hadoop/hive/llap/TestLlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.fs.FileSystem;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.lang.management.ThreadMXBean;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+public class TestLlapThreadLocalStatistics {
+
+ private static final ThreadMXBean mxBean = LlapUtil.initThreadMxBean();
+
+ @Test
+ public void testEmptyStatistics() {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertEquals(0, after.subtract(before).schemeToThreadLocalStats.keySet().size());
+ }
+
+ @Test
+ public void testCpuTimeUserTime() throws Exception {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should be >0", before.cpuTime > 0);
+ Assert.assertTrue("userTime should be >0", before.userTime > 0);
+
+ Thread.sleep(100);
+
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should increase", after.cpuTime > before.cpuTime);
+ // userTime assertion is flaky, not checked here
+ }
+
+ @Test
+ public void testCountersMergedForTheSameScheme() {
+ LlapThreadLocalStatistics stats = new LlapThreadLocalStatistics(mxBean,
+ createMockStatistics(new String[]{"file", "hdfs", "hdfs"}, new Integer[]{1, 1, 1}));
Review Comment:
I don't think so, I cannot recall if we refactor file system schemes to constants in tests, IMO it just leads to more code with no value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2519748439
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856247776
##########
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/StatsRecordingThreadPool.java:
##########
@@ -161,98 +149,44 @@ private void setupMDCFromNDC(final Callable<V> actualCallable) {
}
}
- /**
- * LLAP IO related counters.
- */
- public enum LlapExecutorCounters {
- EXECUTOR_CPU_NS,
- EXECUTOR_USER_NS;
-
- }
-
- private void updateCounters(final List<LlapUtil.StatisticsData> statsBefore,
- final Callable<V> actualCallable, long cpuTime, long userTime) {
+ private void updateCounters(final LlapThreadLocalStatistics statsBefore,
+ final Callable<V> actualCallable) {
Thread thread = Thread.currentThread();
- TezCounters tezCounters = null;
- // add tez counters for task execution and llap io
- if (actualCallable instanceof TaskRunner2Callable) {
- TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
- // counters for task execution side
- tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
- } else if (actualCallable instanceof TezCounterSource) {
- // Other counter sources (currently used in LLAP IO).
- tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
- } else {
- LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
- }
+ final TezCounters tezCounters = getTezCounters(actualCallable);
if (tezCounters != null) {
- if (cpuTime >= 0 && userTime >= 0) {
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
- }
if (statsBefore != null) {
- // if there are multiple stats for the same scheme (from different NameNode), this
- // method will squash them together
- Map<String, FileSystem.Statistics> schemeToStats = LlapUtil
- .getCombinedFileSystemStatistics();
- for (Map.Entry<String, FileSystem.Statistics> entry : schemeToStats.entrySet()) {
- final String scheme = entry.getKey();
- FileSystem.Statistics statistics = entry.getValue();
- FileSystem.Statistics.StatisticsData threadFSStats = statistics
- .getThreadStatistics();
- List<LlapUtil.StatisticsData> allStatsBefore = LlapUtil
- .getStatisticsForScheme(scheme, statsBefore);
- long bytesReadDelta = 0;
- long bytesWrittenDelta = 0;
- long readOpsDelta = 0;
- long largeReadOpsDelta = 0;
- long writeOpsDelta = 0;
- // there could be more scheme after execution as execution might be accessing a
- // different filesystem. So if we don't find a matching scheme before execution we
- // just use the after execution values directly without computing delta difference
- if (allStatsBefore != null && !allStatsBefore.isEmpty()) {
- for (LlapUtil.StatisticsData sb : allStatsBefore) {
- bytesReadDelta += threadFSStats.getBytesRead() - sb.getBytesRead();
- bytesWrittenDelta += threadFSStats.getBytesWritten() - sb.getBytesWritten();
- readOpsDelta += threadFSStats.getReadOps() - sb.getReadOps();
- largeReadOpsDelta += threadFSStats.getLargeReadOps() - sb.getLargeReadOps();
- writeOpsDelta += threadFSStats.getWriteOps() - sb.getWriteOps();
- }
- } else {
- bytesReadDelta = threadFSStats.getBytesRead();
- bytesWrittenDelta = threadFSStats.getBytesWritten();
- readOpsDelta = threadFSStats.getReadOps();
- largeReadOpsDelta = threadFSStats.getLargeReadOps();
- writeOpsDelta = threadFSStats.getWriteOps();
- }
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ)
- .increment(bytesReadDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN)
- .increment(bytesWrittenDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(readOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS)
- .increment(largeReadOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS)
- .increment(writeOpsDelta);
+ LlapThreadLocalStatistics currentStats = new LlapThreadLocalStatistics(mxBean);
+ currentStats.subtract(statsBefore).fill(tezCounters);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} scheme: {} " +
- "bytesRead: {} bytesWritten: {} readOps: {} largeReadOps: {} writeOps: {}",
- actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(),
- scheme, bytesReadDelta, bytesWrittenDelta, readOpsDelta, largeReadOpsDelta,
- writeOpsDelta);
- }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} stats: {}",
+ actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(), currentStats);
}
} else {
LOG.warn("File system statistics snapshot before execution of thread is null." +
- "Thread name: {} id: {} allStats: {}", thread.getName(), thread.getId(),
- statsBefore);
+ "Thread name: {} id: {}", thread.getName(), thread.getId());
}
} else {
LOG.warn("TezCounters is null for callable type: {}",
actualCallable.getClass().getSimpleName());
}
}
}
+
+ private static <V> TezCounters getTezCounters(Callable<V> actualCallable) {
+ TezCounters tezCounters = null;
+ // add tez counters for task execution and llap io
+ if (actualCallable instanceof TaskRunner2Callable) {
+ TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
+ // counters for task execution side
+ tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
+ } else if (actualCallable instanceof TezCounterSource) {
+ // Other counter sources (currently used in LLAP IO).
+ tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
+ } else {
+ LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
Review Comment:
no, this is no expected, but should not turn the task fail
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "zratkai (via GitHub)" <gi...@apache.org>.
zratkai commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856225630
##########
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/StatsRecordingThreadPool.java:
##########
@@ -161,98 +149,44 @@ private void setupMDCFromNDC(final Callable<V> actualCallable) {
}
}
- /**
- * LLAP IO related counters.
- */
- public enum LlapExecutorCounters {
- EXECUTOR_CPU_NS,
- EXECUTOR_USER_NS;
-
- }
-
- private void updateCounters(final List<LlapUtil.StatisticsData> statsBefore,
- final Callable<V> actualCallable, long cpuTime, long userTime) {
+ private void updateCounters(final LlapThreadLocalStatistics statsBefore,
+ final Callable<V> actualCallable) {
Thread thread = Thread.currentThread();
- TezCounters tezCounters = null;
- // add tez counters for task execution and llap io
- if (actualCallable instanceof TaskRunner2Callable) {
- TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
- // counters for task execution side
- tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
- } else if (actualCallable instanceof TezCounterSource) {
- // Other counter sources (currently used in LLAP IO).
- tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
- } else {
- LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
- }
+ final TezCounters tezCounters = getTezCounters(actualCallable);
if (tezCounters != null) {
- if (cpuTime >= 0 && userTime >= 0) {
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
- }
if (statsBefore != null) {
- // if there are multiple stats for the same scheme (from different NameNode), this
- // method will squash them together
- Map<String, FileSystem.Statistics> schemeToStats = LlapUtil
- .getCombinedFileSystemStatistics();
- for (Map.Entry<String, FileSystem.Statistics> entry : schemeToStats.entrySet()) {
- final String scheme = entry.getKey();
- FileSystem.Statistics statistics = entry.getValue();
- FileSystem.Statistics.StatisticsData threadFSStats = statistics
- .getThreadStatistics();
- List<LlapUtil.StatisticsData> allStatsBefore = LlapUtil
- .getStatisticsForScheme(scheme, statsBefore);
- long bytesReadDelta = 0;
- long bytesWrittenDelta = 0;
- long readOpsDelta = 0;
- long largeReadOpsDelta = 0;
- long writeOpsDelta = 0;
- // there could be more scheme after execution as execution might be accessing a
- // different filesystem. So if we don't find a matching scheme before execution we
- // just use the after execution values directly without computing delta difference
- if (allStatsBefore != null && !allStatsBefore.isEmpty()) {
- for (LlapUtil.StatisticsData sb : allStatsBefore) {
- bytesReadDelta += threadFSStats.getBytesRead() - sb.getBytesRead();
- bytesWrittenDelta += threadFSStats.getBytesWritten() - sb.getBytesWritten();
- readOpsDelta += threadFSStats.getReadOps() - sb.getReadOps();
- largeReadOpsDelta += threadFSStats.getLargeReadOps() - sb.getLargeReadOps();
- writeOpsDelta += threadFSStats.getWriteOps() - sb.getWriteOps();
- }
- } else {
- bytesReadDelta = threadFSStats.getBytesRead();
- bytesWrittenDelta = threadFSStats.getBytesWritten();
- readOpsDelta = threadFSStats.getReadOps();
- largeReadOpsDelta = threadFSStats.getLargeReadOps();
- writeOpsDelta = threadFSStats.getWriteOps();
- }
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ)
- .increment(bytesReadDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN)
- .increment(bytesWrittenDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(readOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS)
- .increment(largeReadOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS)
- .increment(writeOpsDelta);
+ LlapThreadLocalStatistics currentStats = new LlapThreadLocalStatistics(mxBean);
+ currentStats.subtract(statsBefore).fill(tezCounters);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} scheme: {} " +
- "bytesRead: {} bytesWritten: {} readOps: {} largeReadOps: {} writeOps: {}",
- actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(),
- scheme, bytesReadDelta, bytesWrittenDelta, readOpsDelta, largeReadOpsDelta,
- writeOpsDelta);
- }
+ if (LOG.isDebugEnabled()) {
Review Comment:
Why do we need to check if debug logging is enabled? I mean LOG.debug logs only when it is enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1860400837
##########
llap-common/src/java/org/apache/hadoop/hive/llap/LlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.tez.common.counters.FileSystemCounter;
+import org.apache.tez.common.counters.TezCounters;
+import java.lang.management.ThreadMXBean;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+
+/**
+ * Convenience class for handling thread local statistics for different schemes in LLAP.
+ * The motivation is that thread local stats (available from FileSystem.getAllStatistics()) are in a List, which
+ * doesn't guarantee that for a single scheme, a single Statistics object is returned (e.g. in case of multiple
+ * namenodes). This class encapsulates that data, and takes care of merging them transparently.
+ * LlapThreadLocalStatistics is used in LLAP's task ThreadPool, where the current thread's statistics is calculated by a
+ * simple delta computation (before/after running the task callable), like:
+ *
+ * LlapThreadLocalStatistics statsBefore = new LlapThreadLocalStatistics(...);
+ * LlapThreadLocalStatistics diff = new LlapThreadLocalStatistics(...).subtract(statsBefore);
+ */
+public class LlapThreadLocalStatistics {
+
+ /**
+ * LLAP IO related counters.
+ */
+ public enum LlapExecutorCounters {
+ EXECUTOR_CPU_NS,
+ EXECUTOR_USER_NS;
+ }
+
+ @VisibleForTesting
+ Map<String, LlapThreadLocalStatistics.StatisticsData> schemeToThreadLocalStats = new HashMap<>();
+ @VisibleForTesting
+ long cpuTime;
+ @VisibleForTesting
+ long userTime;
+
+ /**
+ * In this constructor we create a snapshot of the current thread local statistics and take care of merging the
+ * ones that belong to the same scheme.
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean) {
+ this(mxBean, FileSystem.getAllStatistics());
+ }
+
+ /**
+ * Merges the list to a map.
+ * Input list:
+ * 1. FileSystem.Statistics (scheme: file)
+ * 2. FileSystem.Statistics (scheme: hdfs)
+ * 3. FileSystem.Statistics (scheme: hdfs)
+ * Output map:
+ * file : LlapThreadLocalStatistics.StatisticsData (1)
+ * hdfs : LlapThreadLocalStatistics.StatisticsData (2 + 3)
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean, List<FileSystem.Statistics> allStatistics) {
+ cpuTime = mxBean == null ? -1 : mxBean.getCurrentThreadCpuTime();
+ userTime = mxBean == null ? -1 : mxBean.getCurrentThreadUserTime();
+
+ for (FileSystem.Statistics statistics : allStatistics) {
+ schemeToThreadLocalStats.merge(statistics.getScheme(),
+ new StatisticsData(statistics.getThreadStatistics()),
+ (statsCurrent, statsNew) -> statsCurrent.merge(statistics.getThreadStatistics()));
+ }
+ }
+
+ // This method iterates on the other LlapThreadLocalStatistics's schemes, and subtract them from this one if it's
+ // present here too.
+ public LlapThreadLocalStatistics subtract(LlapThreadLocalStatistics other) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> otherStats :
+ other.schemeToThreadLocalStats.entrySet()){
+ schemeToThreadLocalStats.computeIfPresent(otherStats.getKey(),
+ (thisScheme, stats) -> stats.subtract(otherStats.getValue()));
+ }
+
+ cpuTime -= other.cpuTime;
+ userTime -= other.userTime;
+
+ return this;
+ }
+
+ public void fill(TezCounters tezCounters) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> threadLocalStats :
+ schemeToThreadLocalStats.entrySet()){
+ String scheme = threadLocalStats.getKey();
+ StatisticsData stats = threadLocalStats.getValue();
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ).increment(stats.bytesRead);
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN).increment(stats.bytesWritten);
+ tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(stats.readOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS).increment(stats.largeReadOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS).increment(stats.writeOps);
+ }
+
+ if (cpuTime >= 0 && userTime >= 0) {
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
+ }
+ }
+
+ public String toString(){
+ return String.format("LlapThreadLocalStatistics: %s", schemeToThreadLocalStats.toString());
+ }
+
+ /**
+ * Convenience class over Hadoop's FileSystem.Statistics.StatisticsData.
+ * Unfortunately, neither the fields, nor the convenience methods (e.g. StatisticsData.add, StatisticsData.negate)
+ * are available here as they are package protected, so we cannot reuse them.
+ */
+ public static class StatisticsData {
Review Comment:
renamed to LlapFileSystemStatisticsData, resolving this conversation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2497459365
> Please check my comments!
thanks, responded to those, please let me know if a follow-up needed somewhere
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2496106846
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2519530031
> Added one comment,
>
> LGTM +1
thanks for the review, I just pushed a commit for the constants
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "zratkai (via GitHub)" <gi...@apache.org>.
zratkai commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856228246
##########
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/StatsRecordingThreadPool.java:
##########
@@ -161,98 +149,44 @@ private void setupMDCFromNDC(final Callable<V> actualCallable) {
}
}
- /**
- * LLAP IO related counters.
- */
- public enum LlapExecutorCounters {
- EXECUTOR_CPU_NS,
- EXECUTOR_USER_NS;
-
- }
-
- private void updateCounters(final List<LlapUtil.StatisticsData> statsBefore,
- final Callable<V> actualCallable, long cpuTime, long userTime) {
+ private void updateCounters(final LlapThreadLocalStatistics statsBefore,
+ final Callable<V> actualCallable) {
Thread thread = Thread.currentThread();
- TezCounters tezCounters = null;
- // add tez counters for task execution and llap io
- if (actualCallable instanceof TaskRunner2Callable) {
- TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
- // counters for task execution side
- tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
- } else if (actualCallable instanceof TezCounterSource) {
- // Other counter sources (currently used in LLAP IO).
- tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
- } else {
- LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
- }
+ final TezCounters tezCounters = getTezCounters(actualCallable);
if (tezCounters != null) {
- if (cpuTime >= 0 && userTime >= 0) {
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
- }
if (statsBefore != null) {
- // if there are multiple stats for the same scheme (from different NameNode), this
- // method will squash them together
- Map<String, FileSystem.Statistics> schemeToStats = LlapUtil
- .getCombinedFileSystemStatistics();
- for (Map.Entry<String, FileSystem.Statistics> entry : schemeToStats.entrySet()) {
- final String scheme = entry.getKey();
- FileSystem.Statistics statistics = entry.getValue();
- FileSystem.Statistics.StatisticsData threadFSStats = statistics
- .getThreadStatistics();
- List<LlapUtil.StatisticsData> allStatsBefore = LlapUtil
- .getStatisticsForScheme(scheme, statsBefore);
- long bytesReadDelta = 0;
- long bytesWrittenDelta = 0;
- long readOpsDelta = 0;
- long largeReadOpsDelta = 0;
- long writeOpsDelta = 0;
- // there could be more scheme after execution as execution might be accessing a
- // different filesystem. So if we don't find a matching scheme before execution we
- // just use the after execution values directly without computing delta difference
- if (allStatsBefore != null && !allStatsBefore.isEmpty()) {
- for (LlapUtil.StatisticsData sb : allStatsBefore) {
- bytesReadDelta += threadFSStats.getBytesRead() - sb.getBytesRead();
- bytesWrittenDelta += threadFSStats.getBytesWritten() - sb.getBytesWritten();
- readOpsDelta += threadFSStats.getReadOps() - sb.getReadOps();
- largeReadOpsDelta += threadFSStats.getLargeReadOps() - sb.getLargeReadOps();
- writeOpsDelta += threadFSStats.getWriteOps() - sb.getWriteOps();
- }
- } else {
- bytesReadDelta = threadFSStats.getBytesRead();
- bytesWrittenDelta = threadFSStats.getBytesWritten();
- readOpsDelta = threadFSStats.getReadOps();
- largeReadOpsDelta = threadFSStats.getLargeReadOps();
- writeOpsDelta = threadFSStats.getWriteOps();
- }
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ)
- .increment(bytesReadDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN)
- .increment(bytesWrittenDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(readOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS)
- .increment(largeReadOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS)
- .increment(writeOpsDelta);
+ LlapThreadLocalStatistics currentStats = new LlapThreadLocalStatistics(mxBean);
+ currentStats.subtract(statsBefore).fill(tezCounters);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} scheme: {} " +
- "bytesRead: {} bytesWritten: {} readOps: {} largeReadOps: {} writeOps: {}",
- actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(),
- scheme, bytesReadDelta, bytesWrittenDelta, readOpsDelta, largeReadOpsDelta,
- writeOpsDelta);
- }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} stats: {}",
+ actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(), currentStats);
}
} else {
LOG.warn("File system statistics snapshot before execution of thread is null." +
- "Thread name: {} id: {} allStats: {}", thread.getName(), thread.getId(),
- statsBefore);
+ "Thread name: {} id: {}", thread.getName(), thread.getId());
}
} else {
LOG.warn("TezCounters is null for callable type: {}",
actualCallable.getClass().getSimpleName());
}
}
}
+
+ private static <V> TezCounters getTezCounters(Callable<V> actualCallable) {
+ TezCounters tezCounters = null;
+ // add tez counters for task execution and llap io
+ if (actualCallable instanceof TaskRunner2Callable) {
+ TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
+ // counters for task execution side
+ tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
+ } else if (actualCallable instanceof TezCounterSource) {
+ // Other counter sources (currently used in LLAP IO).
+ tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
+ } else {
+ LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
Review Comment:
Instead of warning, shouldn't an exception thrown?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2519521729
> Added one comment,
>
> LGTM +1
thanks @difin ! responded, let me know if you still think HDFS and FILE constants are needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog merged PR #5557:
URL: https://github.com/apache/hive/pull/5557
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856172835
##########
llap-common/src/java/org/apache/hadoop/hive/llap/LlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.tez.common.counters.FileSystemCounter;
+import org.apache.tez.common.counters.TezCounters;
+import java.lang.management.ThreadMXBean;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+
+/**
+ * Convenience class for handling thread local statistics for different schemes in LLAP.
+ * The motivation is that thread local stats (available from FileSystem.getAllStatistics()) are in a List, which
+ * doesn't guarantee that for a single scheme, a single Statistics object is returned (e.g. in case of multiple
+ * namenodes). This class encapsulates that data, and takes care of merging them transparently.
+ * LlapThreadLocalStatistics is used in LLAP's task ThreadPool, where the current thread's statistics is calculated by a
+ * simple delta computation (before/after running the task callable), like:
+ *
+ * LlapThreadLocalStatistics statsBefore = new LlapThreadLocalStatistics(...);
+ * LlapThreadLocalStatistics diff = new LlapThreadLocalStatistics(...).subtract(statsBefore);
+ */
+public class LlapThreadLocalStatistics {
+
+ /**
+ * LLAP IO related counters.
+ */
+ public enum LlapExecutorCounters {
+ EXECUTOR_CPU_NS,
+ EXECUTOR_USER_NS;
+ }
+
+ @VisibleForTesting
+ Map<String, LlapThreadLocalStatistics.StatisticsData> schemeToThreadLocalStats = new HashMap<>();
+ @VisibleForTesting
+ long cpuTime;
+ @VisibleForTesting
+ long userTime;
+
+ /**
+ * In this constructor we create a snapshot of the current thread local statistics and take care of merging the
+ * ones that belong to the same scheme.
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean) {
+ this(mxBean, FileSystem.getAllStatistics());
+ }
+
+ /**
+ * Merges the list to a map.
+ * Input list:
+ * 1. FileSystem.Statistics (scheme: file)
+ * 2. FileSystem.Statistics (scheme: hdfs)
+ * 3. FileSystem.Statistics (scheme: hdfs)
+ * Output map:
+ * file : LlapThreadLocalStatistics.StatisticsData (1)
+ * hdfs : LlapThreadLocalStatistics.StatisticsData (2 + 3)
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean, List<FileSystem.Statistics> allStatistics) {
+ cpuTime = mxBean == null ? -1 : mxBean.getCurrentThreadCpuTime();
+ userTime = mxBean == null ? -1 : mxBean.getCurrentThreadUserTime();
+
+ for (FileSystem.Statistics statistics : allStatistics) {
+ schemeToThreadLocalStats.merge(statistics.getScheme(),
+ new StatisticsData(statistics.getThreadStatistics()),
+ (statsCurrent, statsNew) -> statsCurrent.merge(statistics.getThreadStatistics()));
+ }
+ }
+
+ // This method iterates on the other LlapThreadLocalStatistics's schemes, and subtract them from this one if it's
+ // present here too.
+ public LlapThreadLocalStatistics subtract(LlapThreadLocalStatistics other) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> otherStats :
+ other.schemeToThreadLocalStats.entrySet()){
+ schemeToThreadLocalStats.computeIfPresent(otherStats.getKey(),
+ (thisScheme, stats) -> stats.subtract(otherStats.getValue()));
+ }
+
+ cpuTime -= other.cpuTime;
+ userTime -= other.userTime;
+
+ return this;
+ }
+
+ public void fill(TezCounters tezCounters) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> threadLocalStats :
+ schemeToThreadLocalStats.entrySet()){
+ String scheme = threadLocalStats.getKey();
+ StatisticsData stats = threadLocalStats.getValue();
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ).increment(stats.bytesRead);
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN).increment(stats.bytesWritten);
+ tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(stats.readOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS).increment(stats.largeReadOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS).increment(stats.writeOps);
+ }
+
+ if (cpuTime >= 0 && userTime >= 0) {
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
+ }
+ }
+
+ public String toString(){
+ return String.format("LlapThreadLocalStatistics: %s", schemeToThreadLocalStats.toString());
+ }
+
+ /**
+ * Convenience class over Hadoop's FileSystem.Statistics.StatisticsData.
+ * Unfortunately, neither the fields, nor the convenience methods (e.g. StatisticsData.add, StatisticsData.negate)
+ * are available here as they are package protected, so we cannot reuse them.
+ */
+ public static class StatisticsData {
Review Comment:
makes sense, I would pick LlapStatisticsData or LlapFileSystemStatisticsData
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "zratkai (via GitHub)" <gi...@apache.org>.
zratkai commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856169633
##########
llap-common/src/java/org/apache/hadoop/hive/llap/LlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.tez.common.counters.FileSystemCounter;
+import org.apache.tez.common.counters.TezCounters;
+import java.lang.management.ThreadMXBean;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+
+/**
+ * Convenience class for handling thread local statistics for different schemes in LLAP.
+ * The motivation is that thread local stats (available from FileSystem.getAllStatistics()) are in a List, which
+ * doesn't guarantee that for a single scheme, a single Statistics object is returned (e.g. in case of multiple
+ * namenodes). This class encapsulates that data, and takes care of merging them transparently.
+ * LlapThreadLocalStatistics is used in LLAP's task ThreadPool, where the current thread's statistics is calculated by a
+ * simple delta computation (before/after running the task callable), like:
+ *
+ * LlapThreadLocalStatistics statsBefore = new LlapThreadLocalStatistics(...);
+ * LlapThreadLocalStatistics diff = new LlapThreadLocalStatistics(...).subtract(statsBefore);
+ */
+public class LlapThreadLocalStatistics {
+
+ /**
+ * LLAP IO related counters.
+ */
+ public enum LlapExecutorCounters {
+ EXECUTOR_CPU_NS,
+ EXECUTOR_USER_NS;
+ }
+
+ @VisibleForTesting
+ Map<String, LlapThreadLocalStatistics.StatisticsData> schemeToThreadLocalStats = new HashMap<>();
+ @VisibleForTesting
+ long cpuTime;
+ @VisibleForTesting
+ long userTime;
+
+ /**
+ * In this constructor we create a snapshot of the current thread local statistics and take care of merging the
+ * ones that belong to the same scheme.
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean) {
+ this(mxBean, FileSystem.getAllStatistics());
+ }
+
+ /**
+ * Merges the list to a map.
+ * Input list:
+ * 1. FileSystem.Statistics (scheme: file)
+ * 2. FileSystem.Statistics (scheme: hdfs)
+ * 3. FileSystem.Statistics (scheme: hdfs)
+ * Output map:
+ * file : LlapThreadLocalStatistics.StatisticsData (1)
+ * hdfs : LlapThreadLocalStatistics.StatisticsData (2 + 3)
+ */
+ public LlapThreadLocalStatistics(ThreadMXBean mxBean, List<FileSystem.Statistics> allStatistics) {
+ cpuTime = mxBean == null ? -1 : mxBean.getCurrentThreadCpuTime();
+ userTime = mxBean == null ? -1 : mxBean.getCurrentThreadUserTime();
+
+ for (FileSystem.Statistics statistics : allStatistics) {
+ schemeToThreadLocalStats.merge(statistics.getScheme(),
+ new StatisticsData(statistics.getThreadStatistics()),
+ (statsCurrent, statsNew) -> statsCurrent.merge(statistics.getThreadStatistics()));
+ }
+ }
+
+ // This method iterates on the other LlapThreadLocalStatistics's schemes, and subtract them from this one if it's
+ // present here too.
+ public LlapThreadLocalStatistics subtract(LlapThreadLocalStatistics other) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> otherStats :
+ other.schemeToThreadLocalStats.entrySet()){
+ schemeToThreadLocalStats.computeIfPresent(otherStats.getKey(),
+ (thisScheme, stats) -> stats.subtract(otherStats.getValue()));
+ }
+
+ cpuTime -= other.cpuTime;
+ userTime -= other.userTime;
+
+ return this;
+ }
+
+ public void fill(TezCounters tezCounters) {
+ for (Map.Entry<String, LlapThreadLocalStatistics.StatisticsData> threadLocalStats :
+ schemeToThreadLocalStats.entrySet()){
+ String scheme = threadLocalStats.getKey();
+ StatisticsData stats = threadLocalStats.getValue();
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ).increment(stats.bytesRead);
+ tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN).increment(stats.bytesWritten);
+ tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(stats.readOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS).increment(stats.largeReadOps);
+ tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS).increment(stats.writeOps);
+ }
+
+ if (cpuTime >= 0 && userTime >= 0) {
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
+ tezCounters.findCounter(LlapThreadLocalStatistics.LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
+ }
+ }
+
+ public String toString(){
+ return String.format("LlapThreadLocalStatistics: %s", schemeToThreadLocalStats.toString());
+ }
+
+ /**
+ * Convenience class over Hadoop's FileSystem.Statistics.StatisticsData.
+ * Unfortunately, neither the fields, nor the convenience methods (e.g. StatisticsData.add, StatisticsData.negate)
+ * are available here as they are package protected, so we cannot reuse them.
+ */
+ public static class StatisticsData {
Review Comment:
I would suggest to rename it which distinct it from FileSystem.Statistics.StatisticsData.
Like HiveStatisticsData or LlapStatisticsData.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2504118137
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2518733484
Added one comment,
LGTM +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1870392838
##########
llap-common/src/test/org/apache/hadoop/hive/llap/TestLlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.fs.FileSystem;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.lang.management.ThreadMXBean;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+public class TestLlapThreadLocalStatistics {
+
+ private static final ThreadMXBean mxBean = LlapUtil.initThreadMxBean();
+
+ @Test
+ public void testEmptyStatistics() {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertEquals(0, after.subtract(before).schemeToThreadLocalStats.keySet().size());
+ }
+
+ @Test
+ public void testCpuTimeUserTime() throws Exception {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should be >0", before.cpuTime > 0);
+ Assert.assertTrue("userTime should be >0", before.userTime > 0);
+
+ Thread.sleep(100);
+
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should increase", after.cpuTime > before.cpuTime);
+ // userTime assertion is flaky, not checked here
+ }
+
+ @Test
+ public void testCountersMergedForTheSameScheme() {
+ LlapThreadLocalStatistics stats = new LlapThreadLocalStatistics(mxBean,
+ createMockStatistics(new String[]{"file", "hdfs", "hdfs"}, new Integer[]{1, 1, 1}));
Review Comment:
"file" and "hdfs" are used in several places in this test class, should they be defined as constants?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1870855491
##########
llap-common/src/test/org/apache/hadoop/hive/llap/TestLlapThreadLocalStatistics.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.hadoop.hive.llap;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.fs.FileSystem;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.lang.management.ThreadMXBean;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+public class TestLlapThreadLocalStatistics {
+
+ private static final ThreadMXBean mxBean = LlapUtil.initThreadMxBean();
+
+ @Test
+ public void testEmptyStatistics() {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertEquals(0, after.subtract(before).schemeToThreadLocalStats.keySet().size());
+ }
+
+ @Test
+ public void testCpuTimeUserTime() throws Exception {
+ LlapThreadLocalStatistics before = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should be >0", before.cpuTime > 0);
+ Assert.assertTrue("userTime should be >0", before.userTime > 0);
+
+ Thread.sleep(100);
+
+ LlapThreadLocalStatistics after = new LlapThreadLocalStatistics(mxBean, new ArrayList<>());
+ Assert.assertTrue("cpuTime should increase", after.cpuTime > before.cpuTime);
+ // userTime assertion is flaky, not checked here
+ }
+
+ @Test
+ public void testCountersMergedForTheSameScheme() {
+ LlapThreadLocalStatistics stats = new LlapThreadLocalStatistics(mxBean,
+ createMockStatistics(new String[]{"file", "hdfs", "hdfs"}, new Integer[]{1, 1, 1}));
Review Comment:
I don't think so, I cannot recall if we refactor file system schemes to constants in tests, IMO it just leads to more code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "zratkai (via GitHub)" <gi...@apache.org>.
zratkai commented on code in PR #5557:
URL: https://github.com/apache/hive/pull/5557#discussion_r1856324242
##########
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/StatsRecordingThreadPool.java:
##########
@@ -161,98 +149,44 @@ private void setupMDCFromNDC(final Callable<V> actualCallable) {
}
}
- /**
- * LLAP IO related counters.
- */
- public enum LlapExecutorCounters {
- EXECUTOR_CPU_NS,
- EXECUTOR_USER_NS;
-
- }
-
- private void updateCounters(final List<LlapUtil.StatisticsData> statsBefore,
- final Callable<V> actualCallable, long cpuTime, long userTime) {
+ private void updateCounters(final LlapThreadLocalStatistics statsBefore,
+ final Callable<V> actualCallable) {
Thread thread = Thread.currentThread();
- TezCounters tezCounters = null;
- // add tez counters for task execution and llap io
- if (actualCallable instanceof TaskRunner2Callable) {
- TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
- // counters for task execution side
- tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
- } else if (actualCallable instanceof TezCounterSource) {
- // Other counter sources (currently used in LLAP IO).
- tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
- } else {
- LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
- }
+ final TezCounters tezCounters = getTezCounters(actualCallable);
if (tezCounters != null) {
- if (cpuTime >= 0 && userTime >= 0) {
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
- }
if (statsBefore != null) {
- // if there are multiple stats for the same scheme (from different NameNode), this
- // method will squash them together
- Map<String, FileSystem.Statistics> schemeToStats = LlapUtil
- .getCombinedFileSystemStatistics();
- for (Map.Entry<String, FileSystem.Statistics> entry : schemeToStats.entrySet()) {
- final String scheme = entry.getKey();
- FileSystem.Statistics statistics = entry.getValue();
- FileSystem.Statistics.StatisticsData threadFSStats = statistics
- .getThreadStatistics();
- List<LlapUtil.StatisticsData> allStatsBefore = LlapUtil
- .getStatisticsForScheme(scheme, statsBefore);
- long bytesReadDelta = 0;
- long bytesWrittenDelta = 0;
- long readOpsDelta = 0;
- long largeReadOpsDelta = 0;
- long writeOpsDelta = 0;
- // there could be more scheme after execution as execution might be accessing a
- // different filesystem. So if we don't find a matching scheme before execution we
- // just use the after execution values directly without computing delta difference
- if (allStatsBefore != null && !allStatsBefore.isEmpty()) {
- for (LlapUtil.StatisticsData sb : allStatsBefore) {
- bytesReadDelta += threadFSStats.getBytesRead() - sb.getBytesRead();
- bytesWrittenDelta += threadFSStats.getBytesWritten() - sb.getBytesWritten();
- readOpsDelta += threadFSStats.getReadOps() - sb.getReadOps();
- largeReadOpsDelta += threadFSStats.getLargeReadOps() - sb.getLargeReadOps();
- writeOpsDelta += threadFSStats.getWriteOps() - sb.getWriteOps();
- }
- } else {
- bytesReadDelta = threadFSStats.getBytesRead();
- bytesWrittenDelta = threadFSStats.getBytesWritten();
- readOpsDelta = threadFSStats.getReadOps();
- largeReadOpsDelta = threadFSStats.getLargeReadOps();
- writeOpsDelta = threadFSStats.getWriteOps();
- }
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ)
- .increment(bytesReadDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN)
- .increment(bytesWrittenDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(readOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS)
- .increment(largeReadOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS)
- .increment(writeOpsDelta);
+ LlapThreadLocalStatistics currentStats = new LlapThreadLocalStatistics(mxBean);
+ currentStats.subtract(statsBefore).fill(tezCounters);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} scheme: {} " +
- "bytesRead: {} bytesWritten: {} readOps: {} largeReadOps: {} writeOps: {}",
- actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(),
- scheme, bytesReadDelta, bytesWrittenDelta, readOpsDelta, largeReadOpsDelta,
- writeOpsDelta);
- }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} stats: {}",
+ actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(), currentStats);
}
} else {
LOG.warn("File system statistics snapshot before execution of thread is null." +
- "Thread name: {} id: {} allStats: {}", thread.getName(), thread.getId(),
- statsBefore);
+ "Thread name: {} id: {}", thread.getName(), thread.getId());
}
} else {
LOG.warn("TezCounters is null for callable type: {}",
actualCallable.getClass().getSimpleName());
}
}
}
+
+ private static <V> TezCounters getTezCounters(Callable<V> actualCallable) {
+ TezCounters tezCounters = null;
+ // add tez counters for task execution and llap io
+ if (actualCallable instanceof TaskRunner2Callable) {
+ TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
+ // counters for task execution side
+ tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
+ } else if (actualCallable instanceof TezCounterSource) {
+ // Other counter sources (currently used in LLAP IO).
+ tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
+ } else {
+ LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
Review Comment:
Ok.
##########
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/StatsRecordingThreadPool.java:
##########
@@ -161,98 +149,44 @@ private void setupMDCFromNDC(final Callable<V> actualCallable) {
}
}
- /**
- * LLAP IO related counters.
- */
- public enum LlapExecutorCounters {
- EXECUTOR_CPU_NS,
- EXECUTOR_USER_NS;
-
- }
-
- private void updateCounters(final List<LlapUtil.StatisticsData> statsBefore,
- final Callable<V> actualCallable, long cpuTime, long userTime) {
+ private void updateCounters(final LlapThreadLocalStatistics statsBefore,
+ final Callable<V> actualCallable) {
Thread thread = Thread.currentThread();
- TezCounters tezCounters = null;
- // add tez counters for task execution and llap io
- if (actualCallable instanceof TaskRunner2Callable) {
- TaskRunner2Callable taskRunner2Callable = (TaskRunner2Callable) actualCallable;
- // counters for task execution side
- tezCounters = taskRunner2Callable.addAndGetTezCounter(FileSystemCounter.class.getName());
- } else if (actualCallable instanceof TezCounterSource) {
- // Other counter sources (currently used in LLAP IO).
- tezCounters = ((TezCounterSource) actualCallable).getTezCounters();
- } else {
- LOG.warn("Unexpected callable {}; cannot get counters", actualCallable);
- }
+ final TezCounters tezCounters = getTezCounters(actualCallable);
if (tezCounters != null) {
- if (cpuTime >= 0 && userTime >= 0) {
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_CPU_NS).increment(cpuTime);
- tezCounters.findCounter(LlapExecutorCounters.EXECUTOR_USER_NS).increment(userTime);
- }
if (statsBefore != null) {
- // if there are multiple stats for the same scheme (from different NameNode), this
- // method will squash them together
- Map<String, FileSystem.Statistics> schemeToStats = LlapUtil
- .getCombinedFileSystemStatistics();
- for (Map.Entry<String, FileSystem.Statistics> entry : schemeToStats.entrySet()) {
- final String scheme = entry.getKey();
- FileSystem.Statistics statistics = entry.getValue();
- FileSystem.Statistics.StatisticsData threadFSStats = statistics
- .getThreadStatistics();
- List<LlapUtil.StatisticsData> allStatsBefore = LlapUtil
- .getStatisticsForScheme(scheme, statsBefore);
- long bytesReadDelta = 0;
- long bytesWrittenDelta = 0;
- long readOpsDelta = 0;
- long largeReadOpsDelta = 0;
- long writeOpsDelta = 0;
- // there could be more scheme after execution as execution might be accessing a
- // different filesystem. So if we don't find a matching scheme before execution we
- // just use the after execution values directly without computing delta difference
- if (allStatsBefore != null && !allStatsBefore.isEmpty()) {
- for (LlapUtil.StatisticsData sb : allStatsBefore) {
- bytesReadDelta += threadFSStats.getBytesRead() - sb.getBytesRead();
- bytesWrittenDelta += threadFSStats.getBytesWritten() - sb.getBytesWritten();
- readOpsDelta += threadFSStats.getReadOps() - sb.getReadOps();
- largeReadOpsDelta += threadFSStats.getLargeReadOps() - sb.getLargeReadOps();
- writeOpsDelta += threadFSStats.getWriteOps() - sb.getWriteOps();
- }
- } else {
- bytesReadDelta = threadFSStats.getBytesRead();
- bytesWrittenDelta = threadFSStats.getBytesWritten();
- readOpsDelta = threadFSStats.getReadOps();
- largeReadOpsDelta = threadFSStats.getLargeReadOps();
- writeOpsDelta = threadFSStats.getWriteOps();
- }
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_READ)
- .increment(bytesReadDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.BYTES_WRITTEN)
- .increment(bytesWrittenDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.READ_OPS).increment(readOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.LARGE_READ_OPS)
- .increment(largeReadOpsDelta);
- tezCounters.findCounter(scheme, FileSystemCounter.WRITE_OPS)
- .increment(writeOpsDelta);
+ LlapThreadLocalStatistics currentStats = new LlapThreadLocalStatistics(mxBean);
+ currentStats.subtract(statsBefore).fill(tezCounters);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Updated stats: instance: {} thread name: {} thread id: {} scheme: {} " +
- "bytesRead: {} bytesWritten: {} readOps: {} largeReadOps: {} writeOps: {}",
- actualCallable.getClass().getSimpleName(), thread.getName(), thread.getId(),
- scheme, bytesReadDelta, bytesWrittenDelta, readOpsDelta, largeReadOpsDelta,
- writeOpsDelta);
- }
+ if (LOG.isDebugEnabled()) {
Review Comment:
Ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2496250226
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28638: Refactor stats handling in StatsRecordingThreadPool [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5557:
URL: https://github.com/apache/hive/pull/5557#issuecomment-2497173215
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5557&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5557&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5557&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5557)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org