You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "hasnain-db (via GitHub)" <gi...@apache.org> on 2023/09/28 05:49:41 UTC
[GitHub] [spark] hasnain-db opened a new pull request, #43165: [SPARK-44937][CORE] Handle InputStream in NettyLogger
hasnain-db opened a new pull request, #43165:
URL: https://github.com/apache/spark/pull/43165
### What changes were proposed in this pull request?
Handle `InputStream`s in the `NettyLogger` so we can print out how many available bytes there are.
### Why are the changes needed?
As part of the SSL support we are going to transfer `InputStream`s via Netty, and this functionality makes it easy to see the size of the streams in the log at a glance.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
CI. Tested as part of the changes in https://github.com/apache/spark/pull/42685 which this is split out of, I observed the logs there.
### Was this patch authored or co-authored using generative AI tooling?
No
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hasnain-db commented on pull request #43165: [SPARK-45377][CORE] Handle InputStream in NettyLogger
Posted by "hasnain-db (via GitHub)" <gi...@apache.org>.
hasnain-db commented on PR #43165:
URL: https://github.com/apache/spark/pull/43165#issuecomment-1740276678
cc: @mridulm @JoshRosen this is now ready to review and has all green tests on CI
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a diff in pull request #43165: [SPARK-45377][CORE] Handle InputStream in NettyLogger
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on code in PR #43165:
URL: https://github.com/apache/spark/pull/43165#discussion_r1342137541
##########
common/network-common/src/main/java/org/apache/spark/network/util/NettyLogger.java:
##########
@@ -42,6 +45,14 @@ protected String format(ChannelHandlerContext ctx, String eventName, Object arg)
} else if (arg instanceof ByteBufHolder) {
return format(ctx, eventName) + " " +
((ByteBufHolder) arg).content().readableBytes() + "B";
+ } else if (arg instanceof InputStream) {
+ int available = 0;
+ try {
+ available = ((InputStream) arg).available();
+ } catch (IOException ex) {
+ // Swallow
Review Comment:
I wonder if we want to show a different number available in this case, like default to -1, to distinguish from "0 available"
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #43165: [SPARK-45377][CORE] Handle InputStream in NettyLogger
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #43165:
URL: https://github.com/apache/spark/pull/43165#discussion_r1341990661
##########
common/network-common/src/main/java/org/apache/spark/network/util/NettyLogger.java:
##########
@@ -42,6 +45,14 @@ protected String format(ChannelHandlerContext ctx, String eventName, Object arg)
} else if (arg instanceof ByteBufHolder) {
return format(ctx, eventName) + " " +
((ByteBufHolder) arg).content().readableBytes() + "B";
+ } else if (arg instanceof InputStream) {
+ int available = 0;
+ try {
+ available = ((InputStream) arg).available();
+ } catch (IOException ex) {
+ // Swallow
+ }
+ return format(ctx, eventName) + " " + available + "B";
Review Comment:
Specific input streams might have additional details that can be useful.
```suggestion
return format(ctx, eventName, arg) + " " + available + "B";
```
##########
common/network-common/src/main/java/org/apache/spark/network/util/NettyLogger.java:
##########
@@ -25,6 +25,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.IOException;
+import java.io.InputStream;
Review Comment:
java imports go first
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #43165: [SPARK-45377][CORE] Handle InputStream in NettyLogger
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #43165:
URL: https://github.com/apache/spark/pull/43165#discussion_r1342194602
##########
common/network-common/src/main/java/org/apache/spark/network/util/NettyLogger.java:
##########
@@ -42,6 +45,15 @@ protected String format(ChannelHandlerContext ctx, String eventName, Object arg)
} else if (arg instanceof ByteBufHolder) {
return format(ctx, eventName) + " " +
((ByteBufHolder) arg).content().readableBytes() + "B";
+ } else if (arg instanceof InputStream) {
+ int available = 0;
Review Comment:
nit: you can default to `-1` and remove the assignment in `catch`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45377][CORE] Handle InputStream in NettyLogger [spark]
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen closed pull request #43165: [SPARK-45377][CORE] Handle InputStream in NettyLogger
URL: https://github.com/apache/spark/pull/43165
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45377][CORE] Handle InputStream in NettyLogger [spark]
Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on PR #43165:
URL: https://github.com/apache/spark/pull/43165#issuecomment-1743018105
Merged to master
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org