You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by pjfanning <gi...@git.apache.org> on 2017/07/17 17:08:23 UTC
[GitHub] spark pull request #18658: [SPARK-20871] only log Janino code at debug level
GitHub user pjfanning opened a pull request:
https://github.com/apache/spark/pull/18658
[SPARK-20871] only log Janino code at debug level
## What changes were proposed in this pull request?
When the code that is generated is greater than 64k, then Janino compile will fail and CodeGenerator.scala will log the entire code at Error level.
SPARK-20871 suggests only logging the code at Debug level.
Since, the code is already logged at debug level, this Pull Request proposes not including the formatted code in the Error logging and exception message at all.
## How was this patch tested?
Existing tests were run.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pjfanning/spark SPARK-20871
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18658.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18658
----
commit 803166c66ed00df88f4f9d629503d1a87c0d3f45
Author: pj.fanning <pj...@workday.com>
Date: 2017-07-17T17:03:18Z
[SPARK-20871] only log Janino code at debug level
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the issue:
https://github.com/apache/spark/pull/18658
That seems reasonable. I'm kind of pro-truncation for very very large code. Even though its not great to have something truncated, outputting GBs of logs is also pretty bad for downstream consumers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128675175
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,25 +1037,25 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
})
try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ logInfo(s"\n${CodeFormatter.format(code, 1000)}")
throw new JaninoRuntimeException(msg, e)
case e: CompileException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ logInfo(s"\n${CodeFormatter.format(code, 1000)}")
--- End diff --
The same here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on the issue:
https://github.com/apache/spark/pull/18658
@ash211 , @marmbrus does this Pull Request look ok or are there other changes needed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79871 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79871/testReport)** for PR 18658 at commit [`c42dc46`](https://github.com/apache/spark/commit/c42dc46653778d600a69f7dd978269492ec7bcde).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79871/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79871 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79871/testReport)** for PR 18658 at commit [`c42dc46`](https://github.com/apache/spark/commit/c42dc46653778d600a69f7dd978269492ec7bcde).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871] only log Janino code at debug level
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the issue:
https://github.com/apache/spark/pull/18658
Jenkins this is ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18658
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] only log Janino code at debug ...
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r127819158
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,24 +1037,22 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
--- End diff --
I'd suggest dropping the string concatenation with `\n` here -- it requires an additional copy of the code to be held in-memory and for errors where the code is too long, this causes unnecessary additional pressure on the heap
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on the issue:
https://github.com/apache/spark/pull/18658
@gatorsmile I made the change you requested.
The idea is that when we log the code when the exceptions happen, that we don't want to print the entire code for very large examples. There is still debug level logging that will print the entire code.
I'm pretty neutral about the limit and if it is necessary at all. My main aim is have the code logging done at a different level from the error message so that I can configure my log settings to not log the code at all. I work on a SaaS solution and we don't our user's code to appear in our logs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79882 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79882/testReport)** for PR 18658 at commit [`a795db2`](https://github.com/apache/spark/commit/a795db225dbdf71efd3cceca906244644e70c218).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79677/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18658
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128906812
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,25 +1037,25 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
})
try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ logInfo(s"\n${CodeFormatter.format(code, maxLines = 1000)}")
--- End diff --
Looks fine to me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79834 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79834/testReport)** for PR 18658 at commit [`252f8ea`](https://github.com/apache/spark/commit/252f8ea36b69ea48a6dfd5dfe6175f49337ee8cf).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79882/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79834/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on the issue:
https://github.com/apache/spark/pull/18658
@gatorsmile I've made the changes you requested.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128717519
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatter.scala ---
@@ -28,14 +28,22 @@ import java.util.regex.Matcher
object CodeFormatter {
val commentHolder = """\/\*(.+?)\*\/""".r
- def format(code: CodeAndComment): String = {
+ def format(code: CodeAndComment): String = format(code, -1)
+
+ def format(code: CodeAndComment, maxLines: Int): String = {
--- End diff --
Can `maxLines` just be an optional argument rather than declare two methods? this is an internal method so its signature is OK to change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79683 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79683/testReport)** for PR 18658 at commit [`52b20f3`](https://github.com/apache/spark/commit/52b20f38f550dacc4896d061c5ac7f69ad56f875).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79830/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128910412
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -562,6 +562,12 @@ object SQLConf {
.intConf
.createWithDefault(20)
+ val CODEGEN_LOGGING_MAX_LINES = buildConf("spark.sql.codegen.logging.maxLines")
+ .internal()
+ .doc("The maximum number of codegen lines to log when errors occur.")
+ .intConf
--- End diff --
Add `checkValue`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871] only log Janino code at debug level
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128910407
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,25 +1038,27 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
})
try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ val maxLines = new SQLConf().loggingMaxLinesForCodegen
+ logInfo(s"\n${CodeFormatter.format(code, maxLines)}")
throw new JaninoRuntimeException(msg, e)
case e: CompileException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ val maxLines = new SQLConf().loggingMaxLinesForCodegen
--- End diff --
-> `SQLConf.get.loggingMaxLinesForCodegen`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18658
When the size is too large, which exception will be thrown?
Truncate for all the cases? or just when the size is too large?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79882 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79882/testReport)** for PR 18658 at commit [`a795db2`](https://github.com/apache/spark/commit/a795db225dbdf71efd3cceca906244644e70c218).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79683 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79683/testReport)** for PR 18658 at commit [`52b20f3`](https://github.com/apache/spark/commit/52b20f38f550dacc4896d061c5ac7f69ad56f875).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79677 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79677/testReport)** for PR 18658 at commit [`803166c`](https://github.com/apache/spark/commit/803166c66ed00df88f4f9d629503d1a87c0d3f45).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on the issue:
https://github.com/apache/spark/pull/18658
@marmbrus How about something like?:
`
val msg = s"failed to compile: $e"
logError(msg, e)
logInfo(formatted)
throw new JaninoRuntimeException(msg, e)
`
Users with info logging enabled would get the code output when an issue arises but users who don't want to log the code could apply a log level of Warn or Error to the class.
If we are going to output the code, then it feels like we should output it all for completeness. I don't have a strong opinion on this latter point though. There is an alternate argument that for large code, then outputting a truncated version might be enough.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128719338
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatter.scala ---
@@ -28,14 +28,22 @@ import java.util.regex.Matcher
object CodeFormatter {
val commentHolder = """\/\*(.+?)\*\/""".r
- def format(code: CodeAndComment): String = {
+ def format(code: CodeAndComment): String = format(code, -1)
+
+ def format(code: CodeAndComment, maxLines: Int): String = {
--- End diff --
@srowen I can make that change. Is there any need to scaladoc the maxLines param? Or is the function definition easy enough to follow?
```
def format(code: CodeAndComment, maxLines: Int = -1): String
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128675159
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,25 +1037,25 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
})
try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ logInfo(s"\n${CodeFormatter.format(code, 1000)}")
--- End diff --
`1000` -> `maxLines = 1000`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79834 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79834/testReport)** for PR 18658 at commit [`252f8ea`](https://github.com/apache/spark/commit/252f8ea36b69ea48a6dfd5dfe6175f49337ee8cf).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128906085
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,25 +1037,25 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
})
try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ logInfo(s"\n${CodeFormatter.format(code, maxLines = 1000)}")
--- End diff --
My only concern whether we should add an internal SQLConf for this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79683/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18658
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128910402
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,25 +1038,27 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
})
try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ val maxLines = new SQLConf().loggingMaxLinesForCodegen
--- End diff --
-> `SQLConf.get.loggingMaxLinesForCodegen`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871] only log Janino code at debug level
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79677 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79677/testReport)** for PR 18658 at commit [`803166c`](https://github.com/apache/spark/commit/803166c66ed00df88f4f9d629503d1a87c0d3f45).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79830 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79830/testReport)** for PR 18658 at commit [`2997ded`](https://github.com/apache/spark/commit/2997ded8c42f6c28faa471136e8c6de0ab7f1e9f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r128906682
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,25 +1037,25 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
})
try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
- val msg = s"failed to compile: $e\n$formatted"
+ val msg = s"failed to compile: $e"
logError(msg, e)
+ logInfo(s"\n${CodeFormatter.format(code, maxLines = 1000)}")
--- End diff --
@gatorsmile I can add a config item to org.apache.spark.sql.internal.SQLConf.scala if you think this is appropriate.
Maybe something like:
```
val CODEGEN_LOGGING_MAX_LINES = buildConf("spark.sql.codegen.logging.maxLines")
.internal()
.doc("The maximum number of codegen lines to log when errors occur.")
.intConf
.createWithDefault(1000)
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the issue:
https://github.com/apache/spark/pull/18658
I don't have super strong opinions here, but in my experience its not always easy to get users to rerun a failed query with a different logging level. Have we considered truncating or special casing the 64k limitation instead?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18658
**[Test build #79830 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79830/testReport)** for PR 18658 at commit [`2997ded`](https://github.com/apache/spark/commit/2997ded8c42f6c28faa471136e8c6de0ab7f1e9f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/18658
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18658: [SPARK-20871][SQL] only log Janino code at debug ...
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on a diff in the pull request:
https://github.com/apache/spark/pull/18658#discussion_r127821957
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -1037,24 +1037,22 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])
- lazy val formatted = CodeFormatter.format(code)
-
logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
- s"\n$formatted"
+ s"\n${CodeFormatter.format(code)}"
--- End diff --
@ash211 the logging will be less user friendly without the \n and this is debug logging - I am working on an enhancement to the CodeFormatter class for the info level logging discussed earlier with @marmbrus that would allow a max number of lines to be specified
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] limit logging of Janino code
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18658
Thanks! Merging to master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18658: [SPARK-20871][SQL] only log Janino code at debug level
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the issue:
https://github.com/apache/spark/pull/18658
FYI for future reviewers as well, we've been running an [extremely similar patch](https://github.com/palantir/spark/pull/181) to PJ's on our distribution of Spark for the past several months and had no problems.
Without this change, a code compilation failure often escalates into a heap OOM as the too-large source code is replicated in memory through the log statement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org