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/07/08 00:34:26 UTC
[GitHub] [hive] dengzhhu653 opened a new pull request, #3386: HIVE-22193: Graceful Shutdown HiveServer2
dengzhhu653 opened a new pull request, #3386:
URL: https://github.com/apache/hive/pull/3386
<!--
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.
-->
1. added unit test
2. start hs2 locally; invoke bin/hive --service hiveserver2 --graceful_stop to stop hs2
the graceful_stop command can also take two parameters, that is, the process id and maximum time(default: 1800s) for killing the process. If absent, the process id will be assigned to the content of $HIVESERVER2_PID_DIR/hiveserver2.pid.
The HIVESERVER2_PID_DIR is default to HIVE_CONF_DIR if not specified.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r925117770
##########
service/src/java/org/apache/hive/service/AbstractService.java:
##########
@@ -125,7 +145,6 @@ public synchronized void stop() {
// started (eg another service failing canceled startup)
return;
}
- ensureCurrentState(STATE.STARTED);
Review Comment:
sure, 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] nrg4878 commented on a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
nrg4878 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r933752080
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -438,7 +439,7 @@ public synchronized void init(HiveConf hiveConf) {
}
// Add a shutdown hook for catching SIGTERM & SIGINT
- ShutdownHookManager.addShutdownHook(() -> hiveServer2.stop());
+ ShutdownHookManager.addShutdownHook(() -> graceful_stop());
Review Comment:
if the default is graceful_stop() when you CNTRL-C the HS2 process, should we decrease the default timeout from 30 mins to something lower? if HS2 does not have any running queries, I think it would be equivalent to a force shutdown. But regardless, just thinking out loud.
if there is a long running query, how would one issue a force shutdown ?
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r909329833
##########
bin/ext/hiveserver2.sh:
##########
@@ -16,24 +16,83 @@
THISSERVICE=hiveserver2
export SERVICE_LIST="${SERVICE_LIST}${THISSERVICE} "
+if [ "$HIVESERVER2_PID_DIR" = "" ]; then
+ HIVESERVER2_PID_DIR=$HIVE_CONF_DIR
+fi
+
+HIVESERVER2_PID=$HIVESERVER2_PID_DIR/hiveserver2.pid
+
+before_start() {
+ #ckeck if the process is not running
+ mkdir -p "$HIVESERVER2_PID_DIR"
+ if [ -f $HIVESERVER2_PID ]; then
+ if kill -0 $(cat $HIVESERVER2_PID) >/dev/null 2>&1; then
+ echo "HiveServer2 running as process $(cat $HIVESERVER2_PID). Stop it first."
+ exit 1
+ fi
+ fi
+}
+
hiveserver2() {
- >&2 echo "$(timestamp): Starting HiveServer2"
- CLASS=org.apache.hive.service.server.HiveServer2
- if $cygwin; then
- HIVE_LIB=`cygpath -w "$HIVE_LIB"`
+ if [ "$1" = "--graceful_stop" ]; then
+ pid=$2
+ if [ "$pid" = "" -a -f $HIVESERVER2_PID ]; then
+ pid=$(cat $HIVESERVER2_PID)
+ fi
+ if [ "$pid" != "" ]; then
+ timeout=$3
+ killAndWait $pid ${timeout:-1800}
Review Comment:
Get the `timeout` value by issuing --getHiveConf 'timeout key' to the hiveserver2, which read it from hive-site.xml
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3386:
URL: https://github.com/apache/hive/pull/3386#issuecomment-1170645262
@hsnusonic @saihemanth-cloudera @sourabh912 clould you please also take a look if have secs? 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 a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919734950
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
Thank you for the review! wait/notify adds extra synchronizing codes in OperationManager. For this case, how about sleeping some time while looping? only a little time, say at most 100ms are wasted for waiting for OperationManager to be done, this does not bring much overhead to graceful stop.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r925118846
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
for timeout purpose, we can seperate the timeout checking from the waiting
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r908007753
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -106,58 +114,64 @@ public ExecuteStatementOperation newExecuteStatementOperation(HiveSession parent
return executeStatementOperation;
}
- public GetTypeInfoOperation newGetTypeInfoOperation(HiveSession parentSession) {
+ public GetTypeInfoOperation newGetTypeInfoOperation(HiveSession parentSession)
+ throws HiveSQLException {
Review Comment:
My idea is that our thrift apis are good at handling the HiveSQLException now, and finally we can see HiveSQLException throwing at the client side even when we create another exception type, so if we can pass the server error message to the client side, it's ok. How about introducing another `errorCode` for this?
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919711845
##########
service/src/java/org/apache/hive/service/AbstractService.java:
##########
@@ -125,7 +145,6 @@ public synchronized void stop() {
// started (eg another service failing canceled startup)
return;
}
- ensureCurrentState(STATE.STARTED);
Review Comment:
only STARTED and DECOMMISSIONED could reach here, other states have been returned directly before this check.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3386:
URL: https://github.com/apache/hive/pull/3386#issuecomment-1199123662
Hello, @sourabh912. Cloud you please take a look at the latest if have a chance? 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 a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r925168540
##########
service/src/java/org/apache/hive/service/CompositeService.java:
##########
@@ -94,6 +94,19 @@ public synchronized void stop() {
super.stop();
}
+ @Override
+ public synchronized void decommission() {
Review Comment:
done
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r908003433
##########
service/src/java/org/apache/hive/service/Service.java:
##########
@@ -63,6 +68,15 @@ public enum STATE {
*/
void start();
+ /**
+ * Shut down the service properly before stopping
Review Comment:
Sure. Thank you for the review!
--
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] sourabh912 commented on pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on PR #3386:
URL: https://github.com/apache/hive/pull/3386#issuecomment-1183792250
@dengzhhu653 : It would be good to add a summary of the changes proposed in this PR.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r920690968
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
Done, thank you!
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
done, 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 closed pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 closed pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
URL: https://github.com/apache/hive/pull/3386
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r908012977
##########
bin/ext/hiveserver2.sh:
##########
@@ -16,24 +16,83 @@
THISSERVICE=hiveserver2
export SERVICE_LIST="${SERVICE_LIST}${THISSERVICE} "
+if [ "$HIVESERVER2_PID_DIR" = "" ]; then
+ HIVESERVER2_PID_DIR=$HIVE_CONF_DIR
+fi
+
+HIVESERVER2_PID=$HIVESERVER2_PID_DIR/hiveserver2.pid
+
+before_start() {
+ #ckeck if the process is not running
+ mkdir -p "$HIVESERVER2_PID_DIR"
+ if [ -f $HIVESERVER2_PID ]; then
+ if kill -0 $(cat $HIVESERVER2_PID) >/dev/null 2>&1; then
+ echo "HiveServer2 running as process $(cat $HIVESERVER2_PID). Stop it first."
+ exit 1
+ fi
+ fi
+}
+
hiveserver2() {
- >&2 echo "$(timestamp): Starting HiveServer2"
- CLASS=org.apache.hive.service.server.HiveServer2
- if $cygwin; then
- HIVE_LIB=`cygpath -w "$HIVE_LIB"`
+ if [ "$1" = "--graceful_stop" ]; then
+ pid=$2
+ if [ "$pid" = "" -a -f $HIVESERVER2_PID ]; then
+ pid=$(cat $HIVESERVER2_PID)
+ fi
+ if [ "$pid" != "" ]; then
+ timeout=$3
+ killAndWait $pid ${timeout:-1800}
Review Comment:
picking from hive-site.xml for `timeout` is more elegant, I searched through xml apis for this purpose, but did not have an answer, another consideration is that one may not even configure the `timeout` in hive-site.xml, so here give a hardcoded default value(30min), and can be changed via `bin/hive --service hiveserver2 --graceful_stop <pid> <timeout>`.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3386:
URL: https://github.com/apache/hive/pull/3386#issuecomment-1183916182
> @dengzhhu653 : It would be good to add a summary of the changes proposed in this PR.
done
--
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] nrg4878 commented on a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
nrg4878 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r907965147
##########
bin/ext/hiveserver2.sh:
##########
@@ -16,24 +16,83 @@
THISSERVICE=hiveserver2
export SERVICE_LIST="${SERVICE_LIST}${THISSERVICE} "
+if [ "$HIVESERVER2_PID_DIR" = "" ]; then
+ HIVESERVER2_PID_DIR=$HIVE_CONF_DIR
+fi
+
+HIVESERVER2_PID=$HIVESERVER2_PID_DIR/hiveserver2.pid
+
+before_start() {
+ #ckeck if the process is not running
+ mkdir -p "$HIVESERVER2_PID_DIR"
+ if [ -f $HIVESERVER2_PID ]; then
+ if kill -0 $(cat $HIVESERVER2_PID) >/dev/null 2>&1; then
+ echo "HiveServer2 running as process $(cat $HIVESERVER2_PID). Stop it first."
+ exit 1
+ fi
+ fi
+}
+
hiveserver2() {
- >&2 echo "$(timestamp): Starting HiveServer2"
- CLASS=org.apache.hive.service.server.HiveServer2
- if $cygwin; then
- HIVE_LIB=`cygpath -w "$HIVE_LIB"`
+ if [ "$1" = "--graceful_stop" ]; then
+ pid=$2
+ if [ "$pid" = "" -a -f $HIVESERVER2_PID ]; then
+ pid=$(cat $HIVESERVER2_PID)
+ fi
+ if [ "$pid" != "" ]; then
+ timeout=$3
+ killAndWait $pid ${timeout:-1800}
Review Comment:
timeout is hardcoded here. I think it should be better to pick this up from the hive-site.xml for HS2. Not sure how we could do that.
##########
bin/ext/hiveserver2.sh:
##########
@@ -16,24 +16,83 @@
THISSERVICE=hiveserver2
export SERVICE_LIST="${SERVICE_LIST}${THISSERVICE} "
+if [ "$HIVESERVER2_PID_DIR" = "" ]; then
+ HIVESERVER2_PID_DIR=$HIVE_CONF_DIR
+fi
+
+HIVESERVER2_PID=$HIVESERVER2_PID_DIR/hiveserver2.pid
+
+before_start() {
+ #ckeck if the process is not running
+ mkdir -p "$HIVESERVER2_PID_DIR"
+ if [ -f $HIVESERVER2_PID ]; then
+ if kill -0 $(cat $HIVESERVER2_PID) >/dev/null 2>&1; then
+ echo "HiveServer2 running as process $(cat $HIVESERVER2_PID). Stop it first."
+ exit 1
+ fi
+ fi
+}
+
hiveserver2() {
- >&2 echo "$(timestamp): Starting HiveServer2"
- CLASS=org.apache.hive.service.server.HiveServer2
- if $cygwin; then
- HIVE_LIB=`cygpath -w "$HIVE_LIB"`
+ if [ "$1" = "--graceful_stop" ]; then
+ pid=$2
+ if [ "$pid" = "" -a -f $HIVESERVER2_PID ]; then
+ pid=$(cat $HIVESERVER2_PID)
+ fi
+ if [ "$pid" != "" ]; then
+ timeout=$3
+ killAndWait $pid ${timeout:-1800}
+ fi
+ else
+ before_start
+ echo >&2 "$(timestamp): Starting HiveServer2"
+ CLASS=org.apache.hive.service.server.HiveServer2
+ if $cygwin; then
+ HIVE_LIB=$(cygpath -w "$HIVE_LIB")
+ fi
+ JAR=${HIVE_LIB}/hive-service-[0-9].*.jar
+
+ export HADOOP_CLIENT_OPTS=" -Dproc_hiveserver2 $HADOOP_CLIENT_OPTS "
+ export HADOOP_OPTS="$HIVESERVER2_HADOOP_OPTS $HADOOP_OPTS"
+ hiveserver2_pid="$$"
Review Comment:
so this would be the PID of the current shell script that starts HS2 right? At this point, we havent launched the HS2 process yet. Dont we need the PID of HS2 process? if so, should HS2 write this file? also, what happens when there are multiple HS2 instances?
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -106,58 +114,64 @@ public ExecuteStatementOperation newExecuteStatementOperation(HiveSession parent
return executeStatementOperation;
}
- public GetTypeInfoOperation newGetTypeInfoOperation(HiveSession parentSession) {
+ public GetTypeInfoOperation newGetTypeInfoOperation(HiveSession parentSession)
+ throws HiveSQLException {
Review Comment:
Should we create a new exception type that more indicative of the state of HS2?
##########
service/src/java/org/apache/hive/service/Service.java:
##########
@@ -63,6 +68,15 @@ public enum STATE {
*/
void start();
+ /**
+ * Shut down the service properly before stopping
Review Comment:
can you add more detailed comment on what happens when this method is called and how 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.
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r909334060
##########
bin/ext/hiveserver2.sh:
##########
@@ -16,24 +16,83 @@
THISSERVICE=hiveserver2
export SERVICE_LIST="${SERVICE_LIST}${THISSERVICE} "
+if [ "$HIVESERVER2_PID_DIR" = "" ]; then
+ HIVESERVER2_PID_DIR=$HIVE_CONF_DIR
+fi
+
+HIVESERVER2_PID=$HIVESERVER2_PID_DIR/hiveserver2.pid
+
+before_start() {
+ #ckeck if the process is not running
+ mkdir -p "$HIVESERVER2_PID_DIR"
+ if [ -f $HIVESERVER2_PID ]; then
+ if kill -0 $(cat $HIVESERVER2_PID) >/dev/null 2>&1; then
+ echo "HiveServer2 running as process $(cat $HIVESERVER2_PID). Stop it first."
+ exit 1
+ fi
+ fi
+}
+
hiveserver2() {
- >&2 echo "$(timestamp): Starting HiveServer2"
- CLASS=org.apache.hive.service.server.HiveServer2
- if $cygwin; then
- HIVE_LIB=`cygpath -w "$HIVE_LIB"`
+ if [ "$1" = "--graceful_stop" ]; then
+ pid=$2
+ if [ "$pid" = "" -a -f $HIVESERVER2_PID ]; then
+ pid=$(cat $HIVESERVER2_PID)
+ fi
+ if [ "$pid" != "" ]; then
+ timeout=$3
+ killAndWait $pid ${timeout:-1800}
+ fi
+ else
+ before_start
+ echo >&2 "$(timestamp): Starting HiveServer2"
+ CLASS=org.apache.hive.service.server.HiveServer2
+ if $cygwin; then
+ HIVE_LIB=$(cygpath -w "$HIVE_LIB")
+ fi
+ JAR=${HIVE_LIB}/hive-service-[0-9].*.jar
+
+ export HADOOP_CLIENT_OPTS=" -Dproc_hiveserver2 $HADOOP_CLIENT_OPTS "
+ export HADOOP_OPTS="$HIVESERVER2_HADOOP_OPTS $HADOOP_OPTS"
+ hiveserver2_pid="$$"
Review Comment:
hiveserver2 also has other commands, such as `--deregister`, `--failover`, those should be seperate from starting hiveserver2, otherwise they may also write its pid into the same file.
--
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] nrg4878 commented on a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
nrg4878 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r940447473
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -94,6 +95,14 @@ public synchronized void start() {
@Override
public synchronized void stop() {
super.stop();
+ for (Operation operation : getOperations()) {
+ try {
Review Comment:
Thanks for the explanation.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919735910
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
Review Comment:
done
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r920691158
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
done, 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 a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r920628083
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -94,6 +95,14 @@ public synchronized void start() {
@Override
public synchronized void stop() {
super.stop();
+ for (Operation operation : getOperations()) {
+ try {
Review Comment:
we actually do not cancel operations at all before the changes, instead we shutdown the backgroud pool running the operations: https://github.com/apache/hive/blob/master/service/src/java/org/apache/hive/service/cli/session/SessionManager.java#L367
this does not guarantee that the HiveServer2 would stop immediately when hs2.stop is finished. In some cases, it would wait for all operations done before exiting, and some operation's local files may not be cleaned.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r920640456
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -94,6 +95,14 @@ public synchronized void start() {
@Override
public synchronized void stop() {
super.stop();
+ for (Operation operation : getOperations()) {
+ try {
Review Comment:
The CliService stops its service on hs2.stop() firstly before OperationManager calling stop(), so it's safe to cancel operations 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.
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 closed pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 closed pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
URL: https://github.com/apache/hive/pull/3386
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r908002850
##########
bin/ext/hiveserver2.sh:
##########
@@ -16,24 +16,83 @@
THISSERVICE=hiveserver2
export SERVICE_LIST="${SERVICE_LIST}${THISSERVICE} "
+if [ "$HIVESERVER2_PID_DIR" = "" ]; then
+ HIVESERVER2_PID_DIR=$HIVE_CONF_DIR
+fi
+
+HIVESERVER2_PID=$HIVESERVER2_PID_DIR/hiveserver2.pid
+
+before_start() {
+ #ckeck if the process is not running
+ mkdir -p "$HIVESERVER2_PID_DIR"
+ if [ -f $HIVESERVER2_PID ]; then
+ if kill -0 $(cat $HIVESERVER2_PID) >/dev/null 2>&1; then
+ echo "HiveServer2 running as process $(cat $HIVESERVER2_PID). Stop it first."
+ exit 1
+ fi
+ fi
+}
+
hiveserver2() {
- >&2 echo "$(timestamp): Starting HiveServer2"
- CLASS=org.apache.hive.service.server.HiveServer2
- if $cygwin; then
- HIVE_LIB=`cygpath -w "$HIVE_LIB"`
+ if [ "$1" = "--graceful_stop" ]; then
+ pid=$2
+ if [ "$pid" = "" -a -f $HIVESERVER2_PID ]; then
+ pid=$(cat $HIVESERVER2_PID)
+ fi
+ if [ "$pid" != "" ]; then
+ timeout=$3
+ killAndWait $pid ${timeout:-1800}
+ fi
+ else
+ before_start
+ echo >&2 "$(timestamp): Starting HiveServer2"
+ CLASS=org.apache.hive.service.server.HiveServer2
+ if $cygwin; then
+ HIVE_LIB=$(cygpath -w "$HIVE_LIB")
+ fi
+ JAR=${HIVE_LIB}/hive-service-[0-9].*.jar
+
+ export HADOOP_CLIENT_OPTS=" -Dproc_hiveserver2 $HADOOP_CLIENT_OPTS "
+ export HADOOP_OPTS="$HIVESERVER2_HADOOP_OPTS $HADOOP_OPTS"
+ hiveserver2_pid="$$"
Review Comment:
> so this would be the PID of the current shell script that starts HS2 right? At this point, we havent launched the HS2 process yet
yeah, that's right. the `exec` command will inherit the process ID of the parent, we also have used in [hive](https://github.com/apache/hive/blob/master/bin/hive#L383) to get the pid of the service process.
> Dont we need the PID of HS2 process? if so, should HS2 write this file? also, what happens when there are multiple HS2 instances?
yes, the pid is used later for killing the process, so we should persist it into a file where locates at $HIVE_CONF_DIR/hiveserver2.pid by default, usually we have a single corresponding `HIVE_CONF_DIR` for each HS2 instance, and this can be changed via env `HIVESERVER2_PID_DIR` when starting the HS2.
Also add a [check](https://github.com/apache/hive/blob/23452342c84cef625b1b89f5e3e9e16bbe9103ad/bin/ext/hiveserver2.sh#L25-L34) here before we start HS2, this avoid mutiple HS2 instances writing its pid to the same file.
--
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] sourabh912 commented on a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r920590934
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
Agreeing that wait/notify may not be required in this case since this code path will only be executed when HS2 is being terminated. I am fine with using sleep. It would be good to add a comment in the code mentioning the same.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919714759
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -208,7 +223,10 @@ private String getQueryId(Operation operation) {
return operation.getQueryId();
}
- private void addOperation(Operation operation) {
+ private void addOperation(Operation operation) throws HiveSQLException {
+ if (getServiceState() != STATE.STARTED) {
Review Comment:
My idea is that only service in STARTED can be allowed to add operations, other states such as INITED should not do this too.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919712742
##########
service/src/java/org/apache/hive/service/Service.java:
##########
@@ -63,6 +69,21 @@ public enum STATE {
*/
void start();
+ /**
+ * Imply the service not to run new requests from client. The service acts accordingly upon
+ * receiving such command. For example:
+ * - HiveServer2: If service discovery is enabled, deregister itself from Zookeeper and
Review Comment:
done, 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 a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919712155
##########
service/src/java/org/apache/hive/service/Service.java:
##########
@@ -39,6 +39,12 @@ public enum STATE {
/** started and not stopped */
STARTED,
+ /**
+ * Telling the service not to run new operations from front users,
+ * but existing queries can still be finished normally
+ */
+ DECOMMISSIONED,
Review Comment:
done, thank you for the comments.
--
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] sourabh912 commented on a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r924864971
##########
service/src/java/org/apache/hive/service/CompositeService.java:
##########
@@ -94,6 +94,19 @@ public synchronized void stop() {
super.stop();
}
+ @Override
+ public synchronized void decommission() {
Review Comment:
nit: add a documentation that services are decommissioned in the reverse order in which they were added.
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,60 @@ private void stopOrDisconnectTezSessions() {
}
}
+ /**
+ * Decommission HiveServer2. As a consequence, SessionManager stops
+ * opening new sessions, OperationManager refuses running new queries and
+ * HiveServer2 deregisters itself from Zookeeper if service discovery is enabled,
+ * but the decommissioning has no effect on the current running queries.
+ */
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (zooKeeperHelper != null && !isDeregisteredWithZooKeeper()) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ // For gracefully stopping, sleeping some time while looping does not bring much overhead,
+ // that is, at most 100ms are wasted for waiting for OperationManager to be done,
+ // and this code path will only be executed when HS2 is being terminated.
+ long sleepInterval = Math.min(100, maxTimeForWait);
+ while (getCliService() != null && getCliService().getSessionManager()
+ .getOperations().size() != 0) {
Review Comment:
Should a better check be
```
while(getCliService() != null && getCliService().getSessionManager() != null && getCliService().getSessionManager().getOperations().size() > 0) {}
```
##########
service/src/java/org/apache/hive/service/CompositeService.java:
##########
@@ -94,6 +94,19 @@ public synchronized void stop() {
super.stop();
}
+ @Override
+ public synchronized void decommission() {
Review Comment:
Also return early if the service is already decommissioned?
```
if (this.getServiceState() == State.DECOMMISSIONING) {
return
}
```
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
Thank you for addressing my comment.
Any particular reason why we need a separate thread to wait on operations to finish? Why can't it be done in same thread calling `graceful_stop` ?
##########
service/src/java/org/apache/hive/service/AbstractService.java:
##########
@@ -125,7 +145,6 @@ public synchronized void stop() {
// started (eg another service failing canceled startup)
return;
}
- ensureCurrentState(STATE.STARTED);
Review Comment:
Understood. Instead of removing the check, shall we change it to one mentioned in my previous comment?
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r926194772
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
Remove the executor, change it to execute in hook thread.
--
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] sourabh912 commented on a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919249214
##########
service/src/java/org/apache/hive/service/AbstractService.java:
##########
@@ -125,7 +145,6 @@ public synchronized void stop() {
// started (eg another service failing canceled startup)
return;
}
- ensureCurrentState(STATE.STARTED);
Review Comment:
Any particular reason for removing this check? I think the check can be modified to
`if (state != STATE.STARTED || state != STATE.DECOMMISSIONED) { throw new IllegalStateException();}`
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -208,7 +223,10 @@ private String getQueryId(Operation operation) {
return operation.getQueryId();
}
- private void addOperation(Operation operation) {
+ private void addOperation(Operation operation) throws HiveSQLException {
+ if (getServiceState() != STATE.STARTED) {
Review Comment:
I think the better check would be
```
if (state == STATE.DECOMMISSIONING || state == STATE.STOPPED) {
throw new HiveSQLException(ErrorMsg.HIVE_SERVER2_INACTIVE, state.name());
}
```
##########
service/src/java/org/apache/hive/service/Service.java:
##########
@@ -39,6 +39,12 @@ public enum STATE {
/** started and not stopped */
STARTED,
+ /**
+ * Telling the service not to run new operations from front users,
+ * but existing queries can still be finished normally
+ */
+ DECOMMISSIONED,
Review Comment:
As a user of this interface, `DECOMMISSIONED` and `STOPPED` states are kind of confusing to me as the states don't define clear boundaries as which state is expected to do what. I think we should rename DECOMMISSIONED to DECOMMISSIONING so that a usual state transition of a service would look like: `STARTED -> DECOMMISSIONING -> STOPPED`
##########
service/src/java/org/apache/hive/service/Service.java:
##########
@@ -63,6 +69,21 @@ public enum STATE {
*/
void start();
+ /**
+ * Imply the service not to run new requests from client. The service acts accordingly upon
+ * receiving such command. For example:
+ * - HiveServer2: If service discovery is enabled, deregister itself from Zookeeper and
Review Comment:
We should move the description for HS2 in HS2's decommission api
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
Review Comment:
should replace this with
```
zooKeeperHelper != null && !isDeregisteredWithZooKeeper()
```
as it ensures to remove instance from zookeeper if not already removed.
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
Instead of doing a busy wait which consumes CPU cycles, a better way would be to handle this via wait/notify mechanism i.e add an api like `waitForOperationsToFinish(waitTimeOut)` in OperationsManager and call invoke it here. The implementation of the api would internally wait for operations count to become zero and notify the caller thread.
--
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] sourabh912 commented on a diff in pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r920597780
##########
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java:
##########
@@ -94,6 +95,14 @@ public synchronized void start() {
@Override
public synchronized void stop() {
super.stop();
+ for (Operation operation : getOperations()) {
+ try {
Review Comment:
Trying to understand how this change is related to decommissioning logic? Before this patch, how are we cancelling operations when hs2.stop is called?
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r919734950
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
Thank you for the review! wait/notify adds extra synchronizing codes in OperationManager. For this case, how about sleeping some time while looping? only a little time, say at most 100ms is wasted for waiting for OperationManager to be done, this does not bring much overhead to graceful stop.
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r933763993
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -438,7 +439,7 @@ public synchronized void init(HiveConf hiveConf) {
}
// Add a shutdown hook for catching SIGTERM & SIGINT
- ShutdownHookManager.addShutdownHook(() -> hiveServer2.stop());
+ ShutdownHookManager.addShutdownHook(() -> graceful_stop());
Review Comment:
Hi @nrg4878, thank you for the comment.
The `graceful_stop` will check both timeout and number of running queries:
https://github.com/apache/hive/blob/746eba21e39965d56d358ef4c195f64054ef27ec/service/src/java/org/apache/hive/service/server/HiveServer2.java#L928
If there's no running queries, then call `stop` to terminate the process with no delay.
For long running queries(timeout happens), then this case will also call `stop`, the operation will cancel the running queries on stopping:
https://github.com/apache/hive/blob/746eba21e39965d56d358ef4c195f64054ef27ec/service/src/java/org/apache/hive/service/cli/operation/OperationManager.java#L95-L105
Besides the command will force shutdown the process when timeout and the process is still alive:
https://github.com/apache/hive/blob/746eba21e39965d56d358ef4c195f64054ef27ec/bin/ext/hiveserver2.sh#L91-L94
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r925127511
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,60 @@ private void stopOrDisconnectTezSessions() {
}
}
+ /**
+ * Decommission HiveServer2. As a consequence, SessionManager stops
+ * opening new sessions, OperationManager refuses running new queries and
+ * HiveServer2 deregisters itself from Zookeeper if service discovery is enabled,
+ * but the decommissioning has no effect on the current running queries.
+ */
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (zooKeeperHelper != null && !isDeregisteredWithZooKeeper()) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ // For gracefully stopping, sleeping some time while looping does not bring much overhead,
+ // that is, at most 100ms are wasted for waiting for OperationManager to be done,
+ // and this code path will only be executed when HS2 is being terminated.
+ long sleepInterval = Math.min(100, maxTimeForWait);
+ while (getCliService() != null && getCliService().getSessionManager()
+ .getOperations().size() != 0) {
Review Comment:
make senses, I'll do that
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r925314897
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,60 @@ private void stopOrDisconnectTezSessions() {
}
}
+ /**
+ * Decommission HiveServer2. As a consequence, SessionManager stops
+ * opening new sessions, OperationManager refuses running new queries and
+ * HiveServer2 deregisters itself from Zookeeper if service discovery is enabled,
+ * but the decommissioning has no effect on the current running queries.
+ */
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (zooKeeperHelper != null && !isDeregisteredWithZooKeeper()) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ // For gracefully stopping, sleeping some time while looping does not bring much overhead,
+ // that is, at most 100ms are wasted for waiting for OperationManager to be done,
+ // and this code path will only be executed when HS2 is being terminated.
+ long sleepInterval = Math.min(100, maxTimeForWait);
+ while (getCliService() != null && getCliService().getSessionManager()
+ .getOperations().size() != 0) {
Review Comment:
Instead of check nullable services, the STATE.STARTED ensures that these two should not be null
--
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 merged pull request #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 merged PR #3386:
URL: https://github.com/apache/hive/pull/3386
--
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 #3386: HIVE-22193: Graceful Shutdown HiveServer2
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3386:
URL: https://github.com/apache/hive/pull/3386#discussion_r925118846
##########
service/src/java/org/apache/hive/service/server/HiveServer2.java:
##########
@@ -895,6 +897,46 @@ private void stopOrDisconnectTezSessions() {
}
}
+ public synchronized void decommission() {
+ LOG.info("Decommissioning HiveServer2");
+ // Remove this server instance from ZooKeeper if dynamic service discovery is set
+ if (serviceDiscovery && !activePassiveHA && zooKeeperHelper != null) {
+ try {
+ zooKeeperHelper.removeServerInstanceFromZooKeeper();
+ } catch (Exception e) {
+ LOG.error("Error removing znode for this HiveServer2 instance from ZooKeeper.", e);
+ }
+ }
+ super.decommission();
+ }
+
+ public synchronized void graceful_stop() {
+ try {
+ decommission();
+ long maxTimeForWait = HiveConf.getTimeVar(getHiveConf(),
+ HiveConf.ConfVars.HIVE_SERVER2_GRACEFUL_STOP_TIMEOUT, TimeUnit.MILLISECONDS);
+ if (maxTimeForWait > 0) {
+ ExecutorService service = Executors.newSingleThreadExecutor();
+ Future future = service.submit(() -> {
+ while (getCliService() != null && getCliService().getSessionManager()
Review Comment:
for timeout purpose, we can seperate the timeout checking from the looping
--
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