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 2022/09/26 18:12:53 UTC
[GitHub] [hive] yigress opened a new pull request, #3621: HIVE-26564: Separate query live operation log and historical operation log
yigress opened a new pull request, #3621:
URL: https://github.com/apache/hive/pull/3621
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### 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'.
-->
### 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
[GitHub] [hive] sunchao commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
sunchao commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1270806065
Merged to master. Thanks @yigress
--
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
[GitHub] [hive] yigress commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
yigress commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1261180092
Thank you @dengzhhu653 for the review! The current test failures are unrelated.
@zabetak as pending reviewer, can you help review? Thank you!
--
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
[GitHub] [hive] dengzhhu653 commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1258882698
> @dengzhhu653 can you help review this? Thank you!
The idea overall makes sense to me, thank you for the contribution! By the way, which kinds of problem do you meet regarding the original design?
--
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
[GitHub] [hive] dengzhhu653 commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1260270179
Hi @yigress, the failed tests seem related, cloud you please fix them?
--
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
[GitHub] [hive] sunchao merged pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
sunchao merged PR #3621:
URL: https://github.com/apache/hive/pull/3621
--
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
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3621:
URL: https://github.com/apache/hive/pull/3621#discussion_r980666461
##########
service/src/test/org/apache/hive/service/cli/operation/TestOperationLogManager.java:
##########
@@ -61,6 +60,7 @@ public void setUp() throws Exception {
HiveConf.setIntVar(hiveConf, HiveConf.ConfVars.HIVE_SERVER2_WEBUI_MAX_HISTORIC_QUERIES, 1);
HiveConf.setIntVar(hiveConf, HiveConf.ConfVars.HIVE_SERVER2_WEBUI_PORT, 8080);
HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_IN_TEST, true);
+ HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_TESTING_REMOVE_LOGS, false);
Review Comment:
why this property should set to false when HIVE_SERVER2_HISTORIC_OPERATION_LOG_ENABLED is enabled? we do not want to set this in production.
--
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
[GitHub] [hive] sfc-gh-aixu commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
sfc-gh-aixu commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1259707761
> @dengzhhu653 can you help review this? Thank you!
+ @pvary and @nrg4878 can you also help review this change?
--
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
[GitHub] [hive] yigress commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
yigress commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1258768746
@dengzhhu653 can you help review this? Thank you!
--
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
[GitHub] [hive] yigress commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
yigress commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1262694824
@zabetak ping ping, latest tests failures are unrelated. Can you help proceed? Thank you!
--
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
[GitHub] [hive] zabetak commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
zabetak commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1262853833
Hey @yigress , I'm still busy with other reviews. Will get back to this ASAP.
--
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
[GitHub] [hive] yigress commented on a diff in pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
yigress commented on code in PR #3621:
URL: https://github.com/apache/hive/pull/3621#discussion_r980701496
##########
service/src/test/org/apache/hive/service/cli/operation/TestOperationLogManager.java:
##########
@@ -61,6 +60,7 @@ public void setUp() throws Exception {
HiveConf.setIntVar(hiveConf, HiveConf.ConfVars.HIVE_SERVER2_WEBUI_MAX_HISTORIC_QUERIES, 1);
HiveConf.setIntVar(hiveConf, HiveConf.ConfVars.HIVE_SERVER2_WEBUI_PORT, 8080);
HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_IN_TEST, true);
+ HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_TESTING_REMOVE_LOGS, false);
Review Comment:
This is because https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/session/OperationLog.java#L80
if (hiveConf.getBoolVar(HiveConf.ConfVars.HIVE_IN_TEST)) {
isRemoveLogs = hiveConf.getBoolVar(HiveConf.ConfVars.HIVE_TESTING_REMOVE_LOGS);
Instead of changing the code there, I thought it may be better set it to false for this test case. In production no need to change this setting.
##########
service/src/java/org/apache/hive/service/cli/operation/Operation.java:
##########
@@ -314,12 +318,13 @@ protected synchronized void cleanupOperationLog(final long operationLogCleanupDe
} else {
if (operationLogCleanupDelayMs > 0) {
ScheduledExecutorService scheduledExecutorService = Executors.newScheduledThreadPool(1);
- scheduledExecutorService.schedule(new OperationLogCleaner(operationLog), operationLogCleanupDelayMs,
+ scheduledExecutorService.schedule(new OperationLogCleaner(this, operationLog), operationLogCleanupDelayMs,
TimeUnit.MILLISECONDS);
scheduledExecutorService.shutdown();
} else {
log.info("Closing operation log {} without delay", operationLog);
operationLog.close();
+ OperationLogManager.closeOperation(this);
Review Comment:
The reason I did not set inside operationLog.close is that OperationLog is a wrapper of the log file itself and has no knowledge of the operation.
--
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
[GitHub] [hive] yigress commented on pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
yigress commented on PR #3621:
URL: https://github.com/apache/hive/pull/3621#issuecomment-1258933010
Thank you @dengzhhu653 so much for your review! The reason for this change is that we want to add a feature to persist the hive QueryInfo and OperationLog at first, for example, persist the information on HDFS or other storage, for ephemeral clusters. Right now only a limited number of historical queries are available in hs2 UI. So OperationLogManager seems an ideal place to do that, or some other ways outside hive that look at the historical operation log location, but current mixing live queries inside historical dir makes it harder to differentiate.
--
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
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3621: HIVE-26564: Separate query live operation log and historical operation log
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3621:
URL: https://github.com/apache/hive/pull/3621#discussion_r980668220
##########
service/src/java/org/apache/hive/service/cli/operation/Operation.java:
##########
@@ -314,12 +318,13 @@ protected synchronized void cleanupOperationLog(final long operationLogCleanupDe
} else {
if (operationLogCleanupDelayMs > 0) {
ScheduledExecutorService scheduledExecutorService = Executors.newScheduledThreadPool(1);
- scheduledExecutorService.schedule(new OperationLogCleaner(operationLog), operationLogCleanupDelayMs,
+ scheduledExecutorService.schedule(new OperationLogCleaner(this, operationLog), operationLogCleanupDelayMs,
TimeUnit.MILLISECONDS);
scheduledExecutorService.shutdown();
} else {
log.info("Closing operation log {} without delay", operationLog);
operationLog.close();
+ OperationLogManager.closeOperation(this);
Review Comment:
nit: can we put `OperationLogManager.closeOperation(this)` into `operationLog.close()`? so we can only take care of SQLOperation's log
--
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