Skip to content

[SPARK-25854][BUILD] fix build/mvn not to fail during Zinc server shutdown #22854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shaneknapp wants to merge 6 commits into from
Closed

[SPARK-25854][BUILD] fix build/mvn not to fail during Zinc server shutdown #22854

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f0a91b
fix mvn to not always exit 1
shaneknapp Oct 26, 2018
d38efe4
simplify and add lightness
shaneknapp Oct 26, 2018
d09f4c0
add jira issue to the comment for future archaeology
shaneknapp Oct 26, 2018
627e7d8
we should exit this script with the mvn exit code
shaneknapp Oct 26, 2018
45bb7b0
removing superfluous true
shaneknapp Oct 26, 2018
3d32b61
updating comments
shaneknapp Oct 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions build/mvn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ if [ -n "${ZINC_INSTALL_FLAG}" -o -z "`"${ZINC_BIN}" -status -port ${ZINC_PORT}`
export ZINC_OPTS=${ZINC_OPTS:-"$_COMPILE_JVM_OPTS"}
"${ZINC_BIN}" -shutdown -port ${ZINC_PORT}
"${ZINC_BIN}" -start -port ${ZINC_PORT} \
-server 127.0.0.1 -idle-timeout 30m \
-server 127.0.0.1 -idle-timeout 3h \
-scala-compiler "${SCALA_COMPILER}" \
-scala-library "${SCALA_LIBRARY}" &>/dev/null
fi
Expand All @@ -163,8 +163,19 @@ export MAVEN_OPTS=${MAVEN_OPTS:-"$_COMPILE_JVM_OPTS"}

echo "Using \`mvn\` from path: $MVN_BIN" 1>&2

# Last, call the `mvn` command as usual
# call the `mvn` command as usual
"${MVN_BIN}" -DzincPort=${ZINC_PORT} "$@"

# Try to shut down zinc explicitly
"${ZINC_BIN}" -shutdown -port ${ZINC_PORT}
# check to see if zinc server is still running post-build
"${ZINC_BIN}" -status -port ${ZINC_PORT} &> /dev/null
ZINC_STATUS=$?

# Try to shut down zinc explicitly if the server is still running
Copy link
Contributor

@cloud-fan cloud-fan Oct 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's very unlikely, but there is a chance that the zinc is timed out between we check its status and shut it down. Since zinc will be timed out eventually, we don't care too much about if we can shut it down successfully here.

So how about "${ZINC_BIN}" -shutdown -port ${ZINC_PORT} || true?

Copy link
Contributor Author

@shaneknapp shaneknapp Oct 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that i'm a couple cups of coffee in to my morning, i'm actually back to thinking that this might be the most elegant way of dealing w/this.

i'll update the PR to do just this, and include a comment describing why we're doing it.

if [ $ZINC_STATUS -eq 0 ]; then
# zinc is still running!
"${ZINC_BIN}" -shutdown -port ${ZINC_PORT}
exit 0
Copy link
Member

@srowen srowen Oct 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose you just want to exit 0 outside the if-else, for clarity, but whatever. This is all fine and can be back-ported back to 2.2

Copy link
Contributor Author

@shaneknapp shaneknapp Oct 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i put the else in there for clarity, and in case we want to ever do something (like report that zinc timed out etc) if the exit code on the status is 1.

¯\(ツ)/¯

i'm also not a fan of putting exit 0 at the end of any bash script, anywhere, ever.

else
# zinc is not running! exit cleanly
exit 0
fi