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 2020/11/16 22:03:00 UTC
[GitHub] [spark] gumartinm opened a new pull request #30390: [SPARK-33463][SQL] Spark Thrift Server, keep Job Id when using incremental collect
gumartinm opened a new pull request #30390:
URL: https://github.com/apache/spark/pull/30390
### What changes were proposed in this pull request?
When enabling **spark.sql.thriftServer.incrementalCollect** Job Ids get lost and tracing queries in Spark Thrift Server ends up being too complicated.
### Why are the changes needed?
Because it will make easier tracing Spark Thrift Server queries.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
The current tests are enough. No need of more tests.
----------------------------------------------------------------
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.
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 #30390: [SPARK-33463][SQL] Spark Thrift Server, keep Job Id when using incremental collect
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30390:
URL: https://github.com/apache/spark/pull/30390#issuecomment-728357007
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.
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 closed pull request #30390: [SPARK-33463][SQL] Keep Job Id during incremental collect in Spark Thrift Server
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #30390:
URL: https://github.com/apache/spark/pull/30390
----------------------------------------------------------------
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.
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] gumartinm commented on a change in pull request #30390: [SPARK-33463][SQL] Spark Thrift Server, keep Job Id when using incremental collect
Posted by GitBox <gi...@apache.org>.
gumartinm commented on a change in pull request #30390:
URL: https://github.com/apache/spark/pull/30390#discussion_r526546753
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkExecuteStatementOperation.scala
##########
@@ -126,6 +126,17 @@ private[hive] class SparkExecuteStatementOperation(
}
def getNextRowSet(order: FetchOrientation, maxRowsL: Long): RowSet = withLocalProperties {
+ try {
+ sqlContext.sparkContext.setJobGroup(statementId, statement)
Review comment:
You are right!!! Thanks a lot for taking the time of revisioning my code.
I implemented the change, hopefully now it looks better. Looking forward to hearing your thoughts.
----------------------------------------------------------------
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.
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 pull request #30390: [SPARK-33463][SQL] Keep Job Id during incremental collect in Spark Thrift Server
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30390:
URL: https://github.com/apache/spark/pull/30390#issuecomment-731602954
@gumartinm . I added you to the Apache Spark contributor group and assigned SPARK-33463 to you.
Thank you for your first contribution again. Welcome to the Apache Spark community.
----------------------------------------------------------------
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.
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 #30390: [SPARK-33463][SQL] Spark Thrift Server, keep Job Id when using incremental collect
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30390:
URL: https://github.com/apache/spark/pull/30390#issuecomment-728356505
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.
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] juliuszsompolski commented on a change in pull request #30390: [SPARK-33463][SQL] Spark Thrift Server, keep Job Id when using incremental collect
Posted by GitBox <gi...@apache.org>.
juliuszsompolski commented on a change in pull request #30390:
URL: https://github.com/apache/spark/pull/30390#discussion_r525928647
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkExecuteStatementOperation.scala
##########
@@ -126,6 +126,17 @@ private[hive] class SparkExecuteStatementOperation(
}
def getNextRowSet(order: FetchOrientation, maxRowsL: Long): RowSet = withLocalProperties {
+ try {
+ sqlContext.sparkContext.setJobGroup(statementId, statement)
Review comment:
minor: in `execute` substitutorStatement is used as a description of the jobgroup. Maybe pull out substitutorStatement to be a class field and use it here as well?
----------------------------------------------------------------
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.
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 change in pull request #30390: [SPARK-33463][SQL] Spark Thrift Server, keep Job Id when using incremental collect
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30390:
URL: https://github.com/apache/spark/pull/30390#discussion_r528215574
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkExecuteStatementOperation.scala
##########
@@ -126,6 +130,17 @@ private[hive] class SparkExecuteStatementOperation(
}
def getNextRowSet(order: FetchOrientation, maxRowsL: Long): RowSet = withLocalProperties {
+ try {
+ sqlContext.sparkContext.setJobGroup(statementId, substitutorStatement)
+ getNextRowSetInternal(order, maxRowsL)
+ } finally {
+ sqlContext.sparkContext.clearJobGroup()
+ }
+ }
+
+ private def getNextRowSetInternal(
+ order: FetchOrientation,
+ maxRowsL: Long): RowSet = withLocalProperties {
Review comment:
Hi, @gumartinm .
Could you rename `maxRowsL` to `maxRows`?
----------------------------------------------------------------
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.
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 removed a comment on pull request #30390: [SPARK-33463][SQL] Spark Thrift Server, keep Job Id when using incremental collect
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30390:
URL: https://github.com/apache/spark/pull/30390#issuecomment-728356505
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.
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