This is an automated email from the ASF dual-hosted git repository.
zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 49e65bdd7fd HIVE-28063: Drop PerfLogger#setPerfLogger method and
unused fields/methods (Stamatis Zampetakis reviewed by Laszlo Bodor, Attila
Turoczy)
49e65bdd7fd is described below
commit 49e65bdd7fd47adffbc59091eaea1618d90c253a
Author: Stamatis Zampetakis <zabe...@gmail.com>
AuthorDate: Tue Feb 6 10:41:17 2024 +0100
HIVE-28063: Drop PerfLogger#setPerfLogger method and unused fields/methods
(Stamatis Zampetakis reviewed by Laszlo Bodor, Attila Turoczy)
The PerfLogger#setPerfLogger is redundant and error-prone.
The small number of current uses could be replaced by simply calling the
respective getter (which implicitly changes the underlying ThreadLocal
variable).
Ideally thread local variable should never be set after obtaining the
initial value. Moreover, allowing any caller to change the thread local
variable can affect the correctness of the program.
Dropping this method improves the encapsulation and readability of the
class.
The org.apache.hadoop.hive.metastore.metrics.PerfLogger has various unused
fields/methods that can be removed as well to improve encapsulation,
readability, and maintenance.
Close apache/hive#5066
---
.../org/apache/hadoop/hive/ql/log/PerfLogger.java | 4 --
.../apache/hadoop/hive/ql/log/PerfLoggerTest.java | 2 -
.../hive/service/cli/operation/SQLOperation.java | 2 +-
.../cli/operation/hplsql/HplSqlOperation.java | 2 +-
.../hadoop/hive/metastore/metrics/PerfLogger.java | 61 ----------------------
5 files changed, 2 insertions(+), 69 deletions(-)
diff --git a/common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
b/common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
index d6fcc0bda75..cc57e9b42b0 100644
--- a/common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
+++ b/common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
@@ -119,10 +119,6 @@ public class PerfLogger {
return result;
}
- public static void setPerfLogger(PerfLogger resetPerfLogger) {
- perfLogger.set(resetPerfLogger);
- }
-
/**
* Call this function when you start to measure time spent by a piece of
code.
* @param callerName the logging object to be used.
diff --git a/common/src/test/org/apache/hadoop/hive/ql/log/PerfLoggerTest.java
b/common/src/test/org/apache/hadoop/hive/ql/log/PerfLoggerTest.java
index d844a983a7b..d06bfa87356 100644
--- a/common/src/test/org/apache/hadoop/hive/ql/log/PerfLoggerTest.java
+++ b/common/src/test/org/apache/hadoop/hive/ql/log/PerfLoggerTest.java
@@ -55,7 +55,6 @@ public class PerfLoggerTest {
AtomicInteger count = new AtomicInteger(0);
// getEndTimes in a loop
executorService.execute(() -> {
- PerfLogger.setPerfLogger(pl);
try {
count.incrementAndGet();
snooze(100);
@@ -76,7 +75,6 @@ public class PerfLoggerTest {
executorService.execute(() -> {
try {
int cnt = count.incrementAndGet();
- PerfLogger.setPerfLogger(pl);
for (int i = 0; i < 64; ++i) {
pl.perfLogBegin("test", PerfLogger.COMPILE + "_ "+ cnt + "_" + i);
snooze(50);
diff --git
a/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
b/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
index 4aa142f52f3..d7deba7b274 100644
--- a/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
+++ b/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
@@ -322,7 +322,7 @@ public class SQLOperation extends ExecuteStatementOperation
{
Hive.set(parentHive);
// TODO: can this result in cross-thread reuse of session state?
SessionState.setCurrentSessionState(parentSessionState);
- PerfLogger.setPerfLogger(SessionState.getPerfLogger());
+ SessionState.getPerfLogger();
if (!embedded) {
LogUtils.registerLoggingContext(queryState.getConf());
}
diff --git
a/service/src/java/org/apache/hive/service/cli/operation/hplsql/HplSqlOperation.java
b/service/src/java/org/apache/hive/service/cli/operation/hplsql/HplSqlOperation.java
index 2bd829fa806..16d16792aed 100644
---
a/service/src/java/org/apache/hive/service/cli/operation/hplsql/HplSqlOperation.java
+++
b/service/src/java/org/apache/hive/service/cli/operation/hplsql/HplSqlOperation.java
@@ -199,7 +199,7 @@ public class HplSqlOperation extends
ExecuteStatementOperation implements Result
assert (!parentHive.allowClose());
Hive.set(parentHive);
SessionState.setCurrentSessionState(parentSessionState);
- PerfLogger.setPerfLogger(SessionState.getPerfLogger());
+ SessionState.getPerfLogger();
LogUtils.registerLoggingContext(queryState.getConf());
ShimLoader.getHadoopShims()
.setHadoopQueryContext(String.format(USER_ID,
queryState.getQueryId(), parentSessionState.getUserName()));
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/PerfLogger.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/PerfLogger.java
index aeede4ee812..923e540f3a2 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/PerfLogger.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/PerfLogger.java
@@ -19,7 +19,6 @@
package org.apache.hadoop.hive.metastore.metrics;
import com.codahale.metrics.Timer;
-import com.google.common.collect.ImmutableMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -38,23 +37,10 @@ public class PerfLogger {
static final private Logger LOG =
LoggerFactory.getLogger(PerfLogger.class.getName());
protected static final ThreadLocal<PerfLogger> perfLogger = new
ThreadLocal<>();
- public static final String GET_AGGR_COL_STATS = "getAggrColStatsFor";
- public static final String GET_AGGR_COL_STATS_2 = "getAggrColStatsFor_2";
- public static final String LIST_PARTS_WITH_AUTH_INFO =
"listPartitionsWithAuthInfo";
- public static final String LIST_PARTS_WITH_AUTH_INFO_2 =
"listPartitionsWithAuthInfo_2";
- public static final String LIST_PARTS_BY_EXPR = "listPartitionsByExpr";
- public static final String LIST_PARTS_SPECS_BY_EXPR =
"listPartitionsSpecByExpr";
public static final String GET_DATABASE = "getDatabase";
public static final String GET_TABLE = "getTable";
- public static final String GET_TABLE_2 = "getTable_2";
- public static final String GET_PK = "getPrimaryKeys";
- public static final String GET_FK = "getForeignKeys";
- public static final String GET_UNIQ_CONSTRAINTS = "getUniqueConstraints";
- public static final String GET_NOT_NULL_CONSTRAINTS =
"getNotNullConstraints";
public static final String GET_TABLE_COL_STATS = "getTableColumnStatistics";
public static final String GET_TABLE_COL_STATS_2 =
"getTableColumnStatistics_2";
- public static final String GET_CONFIG_VAL = "getConfigValue";
-
private PerfLogger() {
// Use getPerfLogger to get an instance of PerfLogger
@@ -79,10 +65,6 @@ public class PerfLogger {
return result;
}
- public static void setPerfLogger(PerfLogger resetPerfLogger) {
- perfLogger.set(resetPerfLogger);
- }
-
/**
* Call this function when you start to measure time spent by a piece of
code.
* @param callerName the logging object to be used.
@@ -138,49 +120,6 @@ public class PerfLogger {
return duration;
}
- public Long getStartTime(String method) {
- long startTime = 0L;
-
- if (startTimes.containsKey(method)) {
- startTime = startTimes.get(method);
- }
- return startTime;
- }
-
- public Long getEndTime(String method) {
- long endTime = 0L;
-
- if (endTimes.containsKey(method)) {
- endTime = endTimes.get(method);
- }
- return endTime;
- }
-
- public boolean startTimeHasMethod(String method) {
- return startTimes.containsKey(method);
- }
-
- public boolean endTimeHasMethod(String method) {
- return endTimes.containsKey(method);
- }
-
- public Long getDuration(String method) {
- long duration = 0;
- if (startTimes.containsKey(method) && endTimes.containsKey(method)) {
- duration = endTimes.get(method) - startTimes.get(method);
- }
- return duration;
- }
-
-
- public ImmutableMap<String, Long> getStartTimes() {
- return ImmutableMap.copyOf(startTimes);
- }
-
- public ImmutableMap<String, Long> getEndTimes() {
- return ImmutableMap.copyOf(endTimes);
- }
-
// Methods for metrics integration. Each thread-local PerfLogger will
open/close scope during each perf-log method.
private transient Map<String, Timer.Context> timerContexts = new HashMap<>();
private transient Timer.Context totalApiCallsTimerContext = null;
