Add help argument for hz-stop script #19749
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hz-devops-test
added
the
Source: Community
This was referenced Oct 10, 2021
frant-hartm
changed the title
frant-hartm
approved these changes
Oct 15, 2021
|
Thanks for the PR! |
|
@frant-hartm Is there anything else I need to do at my end? I don't see a merge option and I am not really sure what your process dictates. |
|
@Chelsea31 Nothing to do on your side, I will get another colleague to review and then we will merge this. |
ldziedziul
reviewed
Oct 20, 2021
| @@ -1,5 +1,12 @@ | |||
| #!/bin/bash | |||
|
|
|||
| if [ "$1" = "help" ] || [ "$1" = "-h" ] || [ "$1" = "--help" ]; then | |||
| echo "Usage : hz-stop" | |||
There was a problem hiding this comment.
What do you think about avoiding the hardcoded script name?
Suggested change
| echo "Usage : hz-stop" | |
| echo "Usage : $0" |
| if [ "$1" = "help" ] || [ "$1" = "-h" ] || [ "$1" = "--help" ]; then | ||
| echo "Usage : hz-stop" | ||
| echo "Used to stop any running instances of HazelcastMemberStarter." | ||
| echo "Finds any processes matching com.hazelcast.core.server.HazelcastMemberStarter and stops all of them." |
There was a problem hiding this comment.
We could extract com.hazelcast.core.server.HazelcastMemberStarter to avoid duplication here:
Suggested change
| echo "Finds any processes matching com.hazelcast.core.server.HazelcastMemberStarter and stops all of them." | |
| echo "Finds any processes matching $PROCESS_MATCHER and stops all of them." |
and here:
PIDS=$(ps ax | grep "$PROCESS_MATCHER" | grep -v grep | awk '{print $1}')There was a problem hiding this comment.
@ldziedziul I have included both of your suggestions in my PR
ldziedziul
approved these changes
Oct 21, 2021
frant-hartm
pushed a commit
to frant-hartm/hazelcast
that referenced
this pull request
Feb 22, 2022
4 tasks
frant-hartm
added a commit
that referenced
this pull request
Mar 3, 2022
* Add hz command to the default distribution package HZ-625 (#20262) * Fix javadoc in distribution module HZ-625 (#20364) * Fix hanging HzStartTest (#20489) The Hazelcast process wasn't being killed correctly and in combination with java.lang.ProcessBuilder#inheritIO the JVM running the test was hanging at shut down (the test passed correctly). There were 2 issues causing improper shutdown: - the scripts hz/hz-start were not using exec command which replaces current shell process with the command, this resulted in killing the shell, but that doesn't immediatelly kill the HazelcastServerCommandLine process. Using exec means we kill the HazelcastServerCommandLine directly. - similarly the HazelcastServerCommandLine starts a new process (HazelcastMemberStarter) and when it is killed it doesn't automatically kill the new process. Added shutdown hook so the process is killed in normal scenario (it won't be killed correclty if `kill -9` is used, but we can't do anything about that). Fixes hazelcast/hazelcast-enterprise#4425 * Add help argument for hz-stop script (#19749) Fixes #19726 * Remove intermediate Java process in hz script [HZ-884] (#20687) Intermediate Java process was used to provide additional classpath or JVM parameters from command line parameters. However, there are issues with the approach: - there are 2 JVMs running, consuming resources - it's not clear to the user what the processes are for, what should be killed - killing with `kill -9` the parent process actually doesn't kill the child process Co-authored-by: Łukasz Dziedziul <lukasz.dziedziul@hazelcast.com> Co-authored-by: Chelsea31 <31530304+Chelsea31@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #19726