-
Notifications
You must be signed in to change notification settings - Fork 232
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.
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
Cleanup shutdown logging for UCX shuffle #1546
Cleanup shutdown logging for UCX shuffle #1546
Conversation
@abellina could you elaborate more on what's being fixed here in the PR description? |
shuffle-plugin/src/main/scala/com/nvidia/spark/rapids/shuffle/ucx/UCX.scala
Outdated
Show resolved
Hide resolved
Would be good to update the PR headline as it's used as-is in the changelog. As it is now, users aren't going to know what it's talking about. |
@jlowe let me know if this looks ok, I'll rebase it if so (and at least sign off the last commit, or squash). It looks like I confused the sign-off bot. |
Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
b84e65a
to
1ca3b13
Compare
build |
1 similar comment
build |
shuffle-plugin/src/main/scala/com/nvidia/spark/rapids/shuffle/ucx/UCX.scala
Outdated
Show resolved
Hide resolved
build |
1 similar comment
build |
Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
build |
@gerashegalov let me know if this looks OK to you. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Cleanup shutdown logging for UCX shuffle Signed-off-by: Alessandro Bellina <abellina@nvidia.com> * Address code review comments Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
* Cleanup shutdown logging for UCX shuffle Signed-off-by: Alessandro Bellina <abellina@nvidia.com> * Address code review comments Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
…IDIA#1546) Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
In this PR we clean up the logging in UCX scala such that we don't log a backtrace in the benign case that the management socket, which is constantly in
accept()
, is closed due to a normal tear down.Java will throw a
SocketException
in this case, to unblock theaccept()
call, and the application must distinguish this exception from other potential exceptions during the call.This PR also cleans up code around an extra lock we had, and makes the
initialized
volatile.Signed-off-by: Alessandro Bellina abellina@nvidia.com
Closes #1155