You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2022/09/28 18:25:12 UTC

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19051


Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................

IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

In HIVE-22193, Hive added graceful shutdown for HiveServer2.
This modified some of the startup scripts to maintain a
pid file. On startup, it verified that the pid is not already
running. Impala uses the hive startup script, but it kills
HiveServer2 with testdata/bin/kill-java-service.sh. If the
pid file outlives the process, then the OS may reuse the
pid and this can cause problems on startup.

This modifies testdata/bin/kill-hive-server.sh to remove
the pid file. testdata/bin/kill-java-service.sh would fail
if it did not kill HiveServer2, so it should be safe to
remove the pid file after the kill-java-service.sh call.

Testing:
 - Verified the hiveserver2.pid file is remove after shutting down.

Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
---
M testdata/bin/kill-hive-server.sh
1 file changed, 5 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/19051/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 1: Code-Review+1

This looks fine. How hard would it to be to use Hive's graceful shutdown?


-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Sep 2022 18:46:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 1: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 03:06:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19051/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19051/1//COMMIT_MSG@23
PS1, Line 23: remove
> typo
Thanks for pointing that out, will fix.



-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 15:34:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 2: Verified+1 Code-Review+2

Only commit message changed, going ahead.


-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 16:11:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

I think I have bumped into the issue in https://gerrit.cloudera.org/#/c/18994/

http://gerrit.cloudera.org:8080/#/c/19051/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19051/1//COMMIT_MSG@23
PS1, Line 23: remove
typo



-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 08:19:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................

IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

In HIVE-22193, Hive added graceful shutdown for HiveServer2.
This modified some of the startup scripts to maintain a
pid file. On startup, it verified that the pid is not already
running. Impala uses the hive startup script, but it kills
HiveServer2 with testdata/bin/kill-java-service.sh. If the
pid file outlives the process, then the OS may reuse the
pid and this can cause problems on startup.

This modifies testdata/bin/kill-hive-server.sh to remove
the pid file. testdata/bin/kill-java-service.sh would fail
if it did not kill HiveServer2, so it should be safe to
remove the pid file after the kill-java-service.sh call.

Testing:
 - Verified the hiveserver2.pid file is removed after shutting down.

Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Reviewed-on: http://gerrit.cloudera.org:8080/19051
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Joe McDonnell <jo...@cloudera.com>
---
M testdata/bin/kill-hive-server.sh
1 file changed, 5 insertions(+), 0 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 1:

> This looks fine. How hard would it to be to use Hive's graceful
 > shutdown?

I don't think it would be hard to switch to using the shutdown logic from the Hive script. At the moment, Hive's graceful shutdown also doesn't remove the pid file. I'm talking to them about it.


-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Sep 2022 18:55:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19051

to look at the new patch set (#2).

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................

IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

In HIVE-22193, Hive added graceful shutdown for HiveServer2.
This modified some of the startup scripts to maintain a
pid file. On startup, it verified that the pid is not already
running. Impala uses the hive startup script, but it kills
HiveServer2 with testdata/bin/kill-java-service.sh. If the
pid file outlives the process, then the OS may reuse the
pid and this can cause problems on startup.

This modifies testdata/bin/kill-hive-server.sh to remove
the pid file. testdata/bin/kill-java-service.sh would fail
if it did not kill HiveServer2, so it should be safe to
remove the pid file after the kill-java-service.sh call.

Testing:
 - Verified the hiveserver2.pid file is removed after shutting down.

Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
---
M testdata/bin/kill-hive-server.sh
1 file changed, 5 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/19051/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11471/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Sep 2022 18:46:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19051 )

Change subject: IMPALA-11621: Remove hiveserver2.pid when shutting down HiveServer2
......................................................................


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8636/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/19051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I813626d06829a86854c6d2c1715f0c5f5109836d
Gerrit-Change-Number: 19051
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Sep 2022 21:54:41 +0000
Gerrit-HasComments: No