New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HIVE-22733 : After disable operation log property in hive, still HS2 saving the operation log #881
Conversation
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."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we dont need this change with the fix for operation log disable in HIVE-22115
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
MDC.remove(QUERYID_LOG_KEY); | ||
MDC.remove(OPERATIONLOG_LEVEL_KEY); | ||
l4j.info("Unregistered logging context."); | ||
if (MDC.get(OPERATIONLOG_LEVEL_KEY) != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this check here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not required ..was for debugging ..will remove it
@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); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we test this with HIVE-22115 backport and see if it works ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean ..without the above changes ? just back porting HIVE-22115 is sufficient ?
…saving the operation log
…saving the operation log : review comment fix
e5b4b21
to
bc8b966
Compare
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
…