You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/01/16 02:03:20 UTC
[GitHub] [hive] maheshk114 opened a new pull request #881: HIVE-22733 :
After disable operation log property in hive,
still HS2 saving the operation log
maheshk114 opened a new pull request #881: HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log
URL: https://github.com/apache/hive/pull/881
…
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] anishek commented on a change in pull request #881:
HIVE-22733 : After disable operation log property in hive,
still HS2 saving the operation log
Posted by GitBox <gi...@apache.org>.
anishek commented on a change in pull request #881: HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log
URL: https://github.com/apache/hive/pull/881#discussion_r367282156
##########
File path: itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingLayout.java
##########
@@ -132,22 +133,47 @@ private void appendHushableRandomAccessFileAppender(Appender queryAppender) {
}
}
- @Test
- public void testSwitchLogLayout() throws Exception {
+ private void executeWithOperationLog(String query, boolean queryLogEnabled) throws Exception {
// verify whether the sql operation log is generated and fetch correctly.
- OperationHandle operationHandle = client.executeStatement(sessionHandle, sqlCntStar, null);
+ OperationHandle operationHandle = client.executeStatement(sessionHandle, query, null);
RowSet rowSetLog = client.fetchResults(operationHandle, FetchOrientation.FETCH_FIRST, 1000,
- FetchType.LOG);
- String queryId = getQueryId(rowSetLog);
- Assert.assertNotNull("Could not find query id, perhaps a logging message changed", queryId);
+ FetchType.LOG);
+ String queryId = "";
+ boolean expectedStopped = true;
+ if (queryLogEnabled) {
+ queryId = getQueryId(rowSetLog);
+ expectedStopped = false;
+ Assert.assertNotNull("Could not find query id, perhaps a logging message changed", queryId);
+ } else {
+ Assert.assertEquals("Operation log is generated even if query logging is disabled", rowSetLog.numRows(), 0);
+ Assert.assertNull("Query id present even if logging is disabled.", getQueryId(rowSetLog));
+ }
- checkAppenderState("before operation close ", LogDivertAppender.QUERY_ROUTING_APPENDER, queryId, false);
- checkAppenderState("before operation close ", LogDivertAppenderForTest.TEST_QUERY_ROUTING_APPENDER, queryId, false);
+ checkAppenderState("before operation close ", LogDivertAppender.QUERY_ROUTING_APPENDER, queryId, expectedStopped);
+ checkAppenderState("before operation close ", LogDivertAppenderForTest.TEST_QUERY_ROUTING_APPENDER, queryId, expectedStopped);
client.closeOperation(operationHandle);
checkAppenderState("after operation close ", LogDivertAppender.QUERY_ROUTING_APPENDER, queryId, true);
checkAppenderState("after operation close ", LogDivertAppenderForTest.TEST_QUERY_ROUTING_APPENDER, queryId, true);
}
+ @Test
+ public void testSwitchLogLayout() throws Exception {
+ executeWithOperationLog(sqlCntStar, true);
+ }
+
+ @Test
+ public void testQueryLogDisabled() throws Exception {
+ OperationHandle operationHandle = client.executeStatement(sessionHandle,
+ "set hive.server2.logging.operation.enabled=false", null);
+ client.closeOperation(operationHandle);
+
+ executeWithOperationLog(sqlCntStar, false);
+
+ operationHandle = client.executeStatement(sessionHandle,
+ "set hive.server2.logging.operation.enabled=true", null);
+ client.closeOperation(operationHandle);
+ }
+
Review comment:
can we test this with HIVE-22115 backport and see if it works ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] anishek commented on a change in pull request #881:
HIVE-22733 : After disable operation log property in hive,
still HS2 saving the operation log
Posted by GitBox <gi...@apache.org>.
anishek commented on a change in pull request #881: HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log
URL: https://github.com/apache/hive/pull/881#discussion_r367281830
##########
File path: common/src/java/org/apache/hadoop/hive/common/LogUtils.java
##########
@@ -219,18 +219,28 @@ public static String maskIfPassword(String key, String value) {
* Register logging context so that log system can print QueryId, SessionId, etc for each message
*/
public static void registerLoggingContext(Configuration conf) {
- MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
- MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
if (HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_ENABLED)) {
+ MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
+ MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
MDC.put(OPERATIONLOG_LEVEL_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL));
+ l4j.info("Thread context registration is done.");
+ } else {
+ l4j.info("Thread context registration is skipped.");
}
}
/**
* Unregister logging context
*/
public static void unregisterLoggingContext() {
- MDC.clear();
+ // Remove the keys added, don't use clear, as it may clear all other things which are not intended to be removed.
+ MDC.remove(SESSIONID_LOG_KEY);
+ MDC.remove(QUERYID_LOG_KEY);
+ MDC.remove(OPERATIONLOG_LEVEL_KEY);
+ l4j.info("Unregistered logging context.");
+ if (MDC.get(OPERATIONLOG_LEVEL_KEY) != null) {
Review comment:
why do we need this check here ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshk114 commented on a change in pull request #881:
HIVE-22733 : After disable operation log property in hive,
still HS2 saving the operation log
Posted by GitBox <gi...@apache.org>.
maheshk114 commented on a change in pull request #881: HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log
URL: https://github.com/apache/hive/pull/881#discussion_r368354709
##########
File path: common/src/java/org/apache/hadoop/hive/common/LogUtils.java
##########
@@ -219,18 +219,28 @@ public static String maskIfPassword(String key, String value) {
* Register logging context so that log system can print QueryId, SessionId, etc for each message
*/
public static void registerLoggingContext(Configuration conf) {
- MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
- MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
if (HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_ENABLED)) {
+ MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
+ MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
MDC.put(OPERATIONLOG_LEVEL_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL));
+ l4j.info("Thread context registration is done.");
+ } else {
+ l4j.info("Thread context registration is skipped.");
}
}
/**
* Unregister logging context
*/
public static void unregisterLoggingContext() {
- MDC.clear();
+ // Remove the keys added, don't use clear, as it may clear all other things which are not intended to be removed.
+ MDC.remove(SESSIONID_LOG_KEY);
+ MDC.remove(QUERYID_LOG_KEY);
+ MDC.remove(OPERATIONLOG_LEVEL_KEY);
+ l4j.info("Unregistered logging context.");
+ if (MDC.get(OPERATIONLOG_LEVEL_KEY) != null) {
Review comment:
not required ..was for debugging ..will remove it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshk114 commented on a change in pull request #881:
HIVE-22733 : After disable operation log property in hive,
still HS2 saving the operation log
Posted by GitBox <gi...@apache.org>.
maheshk114 commented on a change in pull request #881: HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log
URL: https://github.com/apache/hive/pull/881#discussion_r368354644
##########
File path: common/src/java/org/apache/hadoop/hive/common/LogUtils.java
##########
@@ -219,18 +219,28 @@ public static String maskIfPassword(String key, String value) {
* Register logging context so that log system can print QueryId, SessionId, etc for each message
*/
public static void registerLoggingContext(Configuration conf) {
- MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
- MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
if (HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_ENABLED)) {
+ MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
+ MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
MDC.put(OPERATIONLOG_LEVEL_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL));
+ l4j.info("Thread context registration is done.");
+ } else {
+ l4j.info("Thread context registration is skipped.");
Review comment:
i think its required as HIVE-22115 will disable it at init time. This change will make sure that its not registered if the operation log is disabled at session 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] maheshk114 commented on a change in pull request #881:
HIVE-22733 : After disable operation log property in hive,
still HS2 saving the operation log
Posted by GitBox <gi...@apache.org>.
maheshk114 commented on a change in pull request #881: HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log
URL: https://github.com/apache/hive/pull/881#discussion_r368355162
##########
File path: itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingLayout.java
##########
@@ -132,22 +133,47 @@ private void appendHushableRandomAccessFileAppender(Appender queryAppender) {
}
}
- @Test
- public void testSwitchLogLayout() throws Exception {
+ private void executeWithOperationLog(String query, boolean queryLogEnabled) throws Exception {
// verify whether the sql operation log is generated and fetch correctly.
- OperationHandle operationHandle = client.executeStatement(sessionHandle, sqlCntStar, null);
+ OperationHandle operationHandle = client.executeStatement(sessionHandle, query, null);
RowSet rowSetLog = client.fetchResults(operationHandle, FetchOrientation.FETCH_FIRST, 1000,
- FetchType.LOG);
- String queryId = getQueryId(rowSetLog);
- Assert.assertNotNull("Could not find query id, perhaps a logging message changed", queryId);
+ FetchType.LOG);
+ String queryId = "";
+ boolean expectedStopped = true;
+ if (queryLogEnabled) {
+ queryId = getQueryId(rowSetLog);
+ expectedStopped = false;
+ Assert.assertNotNull("Could not find query id, perhaps a logging message changed", queryId);
+ } else {
+ Assert.assertEquals("Operation log is generated even if query logging is disabled", rowSetLog.numRows(), 0);
+ Assert.assertNull("Query id present even if logging is disabled.", getQueryId(rowSetLog));
+ }
- checkAppenderState("before operation close ", LogDivertAppender.QUERY_ROUTING_APPENDER, queryId, false);
- checkAppenderState("before operation close ", LogDivertAppenderForTest.TEST_QUERY_ROUTING_APPENDER, queryId, false);
+ checkAppenderState("before operation close ", LogDivertAppender.QUERY_ROUTING_APPENDER, queryId, expectedStopped);
+ checkAppenderState("before operation close ", LogDivertAppenderForTest.TEST_QUERY_ROUTING_APPENDER, queryId, expectedStopped);
client.closeOperation(operationHandle);
checkAppenderState("after operation close ", LogDivertAppender.QUERY_ROUTING_APPENDER, queryId, true);
checkAppenderState("after operation close ", LogDivertAppenderForTest.TEST_QUERY_ROUTING_APPENDER, queryId, true);
}
+ @Test
+ public void testSwitchLogLayout() throws Exception {
+ executeWithOperationLog(sqlCntStar, true);
+ }
+
+ @Test
+ public void testQueryLogDisabled() throws Exception {
+ OperationHandle operationHandle = client.executeStatement(sessionHandle,
+ "set hive.server2.logging.operation.enabled=false", null);
+ client.closeOperation(operationHandle);
+
+ executeWithOperationLog(sqlCntStar, false);
+
+ operationHandle = client.executeStatement(sessionHandle,
+ "set hive.server2.logging.operation.enabled=true", null);
+ client.closeOperation(operationHandle);
+ }
+
Review comment:
you mean ..without the above changes ? just back porting HIVE-22115 is sufficient ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] anishek commented on a change in pull request #881:
HIVE-22733 : After disable operation log property in hive,
still HS2 saving the operation log
Posted by GitBox <gi...@apache.org>.
anishek commented on a change in pull request #881: HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log
URL: https://github.com/apache/hive/pull/881#discussion_r367281399
##########
File path: common/src/java/org/apache/hadoop/hive/common/LogUtils.java
##########
@@ -219,18 +219,28 @@ public static String maskIfPassword(String key, String value) {
* Register logging context so that log system can print QueryId, SessionId, etc for each message
*/
public static void registerLoggingContext(Configuration conf) {
- MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
- MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
if (HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_ENABLED)) {
+ MDC.put(SESSIONID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVESESSIONID));
+ MDC.put(QUERYID_LOG_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVEQUERYID));
MDC.put(OPERATIONLOG_LEVEL_KEY, HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LEVEL));
+ l4j.info("Thread context registration is done.");
+ } else {
+ l4j.info("Thread context registration is skipped.");
Review comment:
May be we dont need this change with the fix for operation log disable in HIVE-22115
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org