-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-53975][PYTHON] Adds basic Python worker logging support #52689
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
base: master
Are you sure you want to change the base?
Conversation
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 love this!
|
should the log be query-centric instead of worker-centric? How can I find logs for a certain query? |
core/src/main/scala/org/apache/spark/api/python/PythonWorkerLogCapture.scala
Show resolved
Hide resolved
core/src/main/scala/org/apache/spark/api/python/PythonWorkerLogCapture.scala
Show resolved
Hide resolved
core/src/main/scala/org/apache/spark/api/python/PythonWorkerLogCapture.scala
Show resolved
Hide resolved
| - func_name: Name of the function that initiated the logging | ||
| - class_name: Name of the class that initiated the logging if available |
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.
do we consider add module name into context?
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 think we can add it if necessary.
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.
This is super awesome! Thank for working on it!
| writer.writeLog( | ||
| PythonWorkerLogLine(System.currentTimeMillis(), seqId.getAndIncrement(), json) |
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.
Do we want to limit the number of lines written to block manager?
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.
cc @ivoson
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.
cc @ivoson @cloud-fan this is important as we don't want users to write unlimited number of logs into block manager.
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.
should the log be query-centric instead of worker-centric? How can I find logs for a certain query?
Do we have any info to identify a query, like query_id in the executor?
If we have, I can add it to context, then we can query with context.query_id = 'xxx'.
core/src/main/scala/org/apache/spark/api/python/PythonWorkerLogCapture.scala
Show resolved
Hide resolved
core/src/main/scala/org/apache/spark/api/python/PythonWorkerLogCapture.scala
Show resolved
Hide resolved
core/src/main/scala/org/apache/spark/api/python/PythonWorkerLogCapture.scala
Show resolved
Hide resolved
| writer.writeLog( | ||
| PythonWorkerLogLine(System.currentTimeMillis(), seqId.getAndIncrement(), json) |
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.
cc @ivoson
| - func_name: Name of the function that initiated the logging | ||
| - class_name: Name of the class that initiated the logging if available |
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 think we can add it if necessary.
core/src/main/scala/org/apache/spark/api/python/PythonWorkerFactory.scala
Outdated
Show resolved
Hide resolved
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.
+1, LGTM (Pending CIs). Thank you for updating the PR.
Please rebase the PR to the master branch because master branch was broken Today for a while and recovered now, @ueshin .
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.
Looks great! Very excited for this feature!
| writer.writeLog( | ||
| PythonWorkerLogLine(System.currentTimeMillis(), seqId.getAndIncrement(), json) |
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.
cc @ivoson @cloud-fan this is important as we don't want users to write unlimited number of logs into block manager.
|
cc @cloud-fan for another look. |
What changes were proposed in this pull request?
Adds basic Python worker logging support.
The logs from Python's standard logger or
printtostdoutandstderrwill be in thesystem.session.python_worker_logsview.spark.sql.pyspark.worker.logging.enabled(Falseby default)When set to true, this configuration enables comprehensive logging within Python worker processes that execute User-Defined Functions (UDFs), User-Defined Table Functions (UDTFs), and other Python-based operations in Spark SQL.
For example:
Why are the changes needed?
The logging in UDF is difficult to collect the logs as they will go to the executor's
stderrfile.If there are many executors, need to check the
stderrfiles one-by-one.Does this PR introduce any user-facing change?
Yes, Python UDF logging is available and collect them via a system view.
How was this patch tested?
Added the related tests.
Was this patch authored or co-authored using generative AI tooling?
No.