You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/09 07:01:08 UTC
[GitHub] [spark] idealspark opened a new pull request, #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
idealspark opened a new pull request, #38993:
URL: https://github.com/apache/spark/pull/38993
### Why are the changes needed?
<!--
fix spark thrift server operation log output is empty
-->
### Does this PR introduce _any_ user-facing change?
<!--
'No'.
-->
### How was this patch tested?
<!--
tests were not added, it was difficult to add.
-->
--
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] idealspark commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
idealspark commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1045027221
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
Review Comment:
i have restored
--
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] idealspark commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
idealspark commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1044969362
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode, CharArrayWriter writer) {
Review Comment:
this is no way to create CharArrayWriter in original constructor.
because original constructor first need call supper constructor. supper constructor need a CharArrayWriter as parameter.
```
private LogDivertAppender(OperationManager operationManager,
OperationLog.LoggingLevel loggingMode, CharArrayWriter writer) {
super("LogDivertAppender", initLayout(loggingMode), null, false, true, Property.EMPTY_ARRAY,
new WriterManager(writer, "LogDivertAppender",
initLayout(loggingMode), true));
}
```
if create CharArrayWriter in original constructor like you suggest compile will failed.
--
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] viirya commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1044738513
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
Review Comment:
Any reason to change it to private constructor and add `create` for constructing?
--
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] dongjoon-hyun commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1045023941
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode, CharArrayWriter writer) {
Review Comment:
Two more spaces like before?
--
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] viirya commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1044737028
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode, CharArrayWriter writer) {
super("LogDivertAppender", initLayout(loggingMode), null, false, true, Property.EMPTY_ARRAY,
- new WriterManager(new CharArrayWriter(), "LogDivertAppender",
Review Comment:
The issue is here using another `CharArrayWriter`?
--
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] AmplabJenkins commented on pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #38993:
URL: https://github.com/apache/spark/pull/38993#issuecomment-1345267189
Can one of the admins verify this patch?
--
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] viirya commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1044982387
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode, CharArrayWriter writer) {
Review Comment:
Oh, yea. I didn't run the suggested one.
--
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] idealspark commented on pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
idealspark commented on PR #38993:
URL: https://github.com/apache/spark/pull/38993#issuecomment-1345181583
>
i restore it.
--
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] viirya commented on pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #38993:
URL: https://github.com/apache/spark/pull/38993#issuecomment-1345347019
Merged to master. There are conflicts on branch-3.3. Can you submit a backport PR to branch-3.3?
--
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] viirya commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1044740898
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode, CharArrayWriter writer) {
Review Comment:
Isn't it just to create `CharArrayWriter` in original constructor?
```suggestion
public LogDivertAppender(OperationManager operationManager,
OperationLog.LoggingLevel loggingMode) {
CharArrayWriter writer = new CharArrayWriter();
```
--
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] dongjoon-hyun commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1045023902
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
Review Comment:
Please restore the original indentation.
--
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] LuciferYang commented on pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38993:
URL: https://github.com/apache/spark/pull/38993#issuecomment-1344370732
cc @viirya FYI
--
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] viirya commented on pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #38993:
URL: https://github.com/apache/spark/pull/38993#issuecomment-1344649595
BTW, seems you change the PR description template by mistake. Can you restore the template?
--
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] idealspark commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
idealspark commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1044964246
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode, CharArrayWriter writer) {
super("LogDivertAppender", initLayout(loggingMode), null, false, true, Property.EMPTY_ARRAY,
- new WriterManager(new CharArrayWriter(), "LogDivertAppender",
Review Comment:
yes, it use another CharArrayWriter, and in this class append() method does not call supper.append() so CharArrayWriter is empty.
--
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] idealspark commented on pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
idealspark commented on PR #38993:
URL: https://github.com/apache/spark/pull/38993#issuecomment-1345553260
> Thanks. Merged to master. There are conflicts on branch-3.3. Can you submit a backport PR to branch-3.3?
ok
--
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] viirya commented on pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #38993:
URL: https://github.com/apache/spark/pull/38993#issuecomment-1345117121
> BTW, seems you change the PR description template by mistake. Can you restore the template?
Can you restore to the standard template?
--
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] idealspark commented on a diff in pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
idealspark commented on code in PR #38993:
URL: https://github.com/apache/spark/pull/38993#discussion_r1044970950
##########
sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/operation/LogDivertAppender.java:
##########
@@ -276,12 +276,19 @@ private static StringLayout initLayout(OperationLog.LoggingLevel loggingMode) {
return getLayout(isVerbose, layout);
}
- public LogDivertAppender(OperationManager operationManager,
- OperationLog.LoggingLevel loggingMode) {
+ public static LogDivertAppender create(OperationManager operationManager,
+ OperationLog.LoggingLevel loggingMode) {
+ CharArrayWriter writer = new CharArrayWriter();
+ return new LogDivertAppender(operationManager, loggingMode, writer);
+ }
+
+ private LogDivertAppender(OperationManager operationManager,
Review Comment:
we need CharArrayWriter as parameter pass to LogDivertAppender constructor , then pass to supper constructor.
--
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] viirya closed pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
Posted by GitBox <gi...@apache.org>.
viirya closed pull request #38993: [SPARK-41459][SQL] fix thrift server operation log output is empty
URL: https://github.com/apache/spark/pull/38993
--
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