-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-20871][SQL] limit logging of Janino code #18658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
803166c
52b20f3
2997ded
252f8ea
c42dc46
a795db2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ import org.apache.spark.metrics.source.CodegenMetrics | |
| import org.apache.spark.sql.catalyst.InternalRow | ||
| import org.apache.spark.sql.catalyst.expressions._ | ||
| import org.apache.spark.sql.catalyst.util.{ArrayData, MapData} | ||
| import org.apache.spark.sql.internal.SQLConf | ||
| import org.apache.spark.sql.types._ | ||
| import org.apache.spark.unsafe.Platform | ||
| import org.apache.spark.unsafe.types._ | ||
|
|
@@ -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 | ||
|
||
| logInfo(s"\n${CodeFormatter.format(code, maxLines)}") | ||
| throw new CompileException(msg, e.getLocation) | ||
| } | ||
| evaluator.getClazz().newInstance().asInstanceOf[GeneratedClass] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add |
||
| .createWithDefault(1000) | ||
|
|
||
| val FILES_MAX_PARTITION_BYTES = buildConf("spark.sql.files.maxPartitionBytes") | ||
| .doc("The maximum number of bytes to pack into a single partition when reading files.") | ||
| .longConf | ||
|
|
@@ -1002,6 +1008,8 @@ class SQLConf extends Serializable with Logging { | |
|
|
||
| def maxCaseBranchesForCodegen: Int = getConf(MAX_CASES_BRANCHES) | ||
|
|
||
| def loggingMaxLinesForCodegen: Int = getConf(CODEGEN_LOGGING_MAX_LINES) | ||
|
|
||
| def tableRelationCacheSize: Int = | ||
| getConf(StaticSQLConf.FILESOURCE_TABLE_RELATION_CACHE_SIZE) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest dropping the string concatenation with
\nhere -- 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 heapThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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