JoshRosen commented on code in PR #49350:
URL: https://github.com/apache/spark/pull/49350#discussion_r1908096993
##########
common/network-common/src/main/java/org/apache/spark/network/server/TransportServer.java:
##########
@@ -151,7 +153,10 @@ protected void initChannel(SocketChannel ch) {
InetSocketAddress localAddress = (InetSocketAddress)
channelFuture.channel().localAddress();
port = localAddress.getPort();
- logger.debug("Shuffle server started on {} with port {}",
localAddress.getHostString(), port);
+ logger.info("{} server started on {} with port {}",
Review Comment:
I'm not necessarily opposed to bundling it in this PR, though:
- This log is only logged once per shuffle server boot, which ordinarily a
once-per-JVM / service operation, so raising its level to `info` seems fine
from a log noise perspective.
- If we're adding the ability to run this in local-cluster mode using
dynamic ports then including ports in the log could be helpful when debugging
testing issues.
- As long as we're updating it / changing it anyways, updating the call site
to use the [new convention for structured logging
interpolation](https://github.com/apache/spark/blob/bbb8aca0b51008bf65ba8f9232ba96c166e84f8e/common/utils/src/main/java/org/apache/spark/internal/SparkLogger.java#L32-L74)
seems fine to me.
We eventually still might want to go back and MDC-ify the other logs, but I
think that can be done separately from this one highly-useful log that's
tackled 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
