[SPARK-45852][CONNECT][PYTHON] Gracefully deal with recursion error during logging#43732
Closed
grundprinzip wants to merge 2 commits intoapache:masterfrom
Closed
[SPARK-45852][CONNECT][PYTHON] Gracefully deal with recursion error during logging#43732grundprinzip wants to merge 2 commits intoapache:masterfrom
grundprinzip wants to merge 2 commits intoapache:masterfrom
Conversation
HyukjinKwon
approved these changes
Nov 10, 2023
Member
|
Merged to master. |
|
|
||
|
|
||
| class SparkConnectBasicTests(SparkConnectSQLTestCase): | ||
| def test_recursion_handling_for_plan_logging(self): |
Member
There was a problem hiding this comment.
Hi, @grundprinzip and @HyukjinKwon .
This test case seems to fail with Python 3.11. SPARK-45987 is filed for Python 3.11 failure.
|
|
||
| # Calling schema will trigger logging the message that will in turn trigger the message | ||
| # conversion into protobuf that will then trigger the recursion error. | ||
| self.assertIsNotNone(cdf.schema) |
Member
There was a problem hiding this comment.
The failure happens here.
ERROR [3.874s]: test_recursion_handling_for_plan_logging (pyspark.sql.tests.connect.test_connect_basic.SparkConnectBasicTests.test_recursion_handling_for_plan_logging)
SPARK-45852 - Test that we can handle recursion in plan logging.
----------------------------------------------------------------------
Traceback (most recent call last):
File "/__w/spark/spark/python/pyspark/sql/tests/connect/test_connect_basic.py", line 171, in test_recursion_handling_for_plan_logging
self.assertIsNotNone(cdf.schema)
^^^^^^^^^^
File "/__w/spark/spark/python/pyspark/sql/connect/dataframe.py", line 1735, in schema
return self._session.client.schema(query)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/__w/spark/spark/python/pyspark/sql/connect/client/core.py", line 924, in schema
schema = self._analyze(method="schema", plan=plan).schema
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/__w/spark/spark/python/pyspark/sql/connect/client/core.py", line 1110, in _analyze
self._handle_error(error)
File "/__w/spark/spark/python/pyspark/sql/connect/client/core.py", line 1499, in _handle_error
self._handle_rpc_error(error)
File "/__w/spark/spark/python/pyspark/sql/connect/client/core.py", line 1570, in _handle_rpc_error
raise SparkConnectGrpcException(str(rpc_error)) from None
pyspark.errors.exceptions.connect.SparkConnectGrpcException: <_InactiveRpcError of RPC that terminated with:
status = StatusCode.INTERNAL
details = "Exception serializing request!"
debug_error_string = "None"
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
The Python client for Spark connect logs the text representation of the proto message. However, for deeply nested objects this can lead to a Python recursion error even before the maximum nested recursion limit of the GRPC message is reached.
This patch fixes this issue by explicitly catching the recursion error during text conversion.
Why are the changes needed?
Stability
Does this PR introduce any user-facing change?
No
How was this patch tested?
UT
Was this patch authored or co-authored using generative AI tooling?
No