Skip to content
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

[SPARK-50338][CORE] Make LazyTry exceptions less verbose #48882

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

juliuszsompolski
Copy link
Contributor

@juliuszsompolski juliuszsompolski commented Nov 18, 2024

What changes were proposed in this pull request?

For LazyTry, we want two stack traces:

  1. the one from the original execution when the exception happened
  2. the one from the current execution, that retrieved the exception.

The first one is added as suppressed exception. The second one is stitched into the stack trace of the rethrown exception.

The third one (TRY_WITH_CALLER_STACKTRACE_TRY_STACKTRACE) doesn't need to be visible to users as a suppressed exception. We only need the depth of the stacktrace until doTryWithCallerStacktrace. Use a custom exception OriginalTryStackTraceException to store it together with the suppressed first one.

Why are the changes needed?

The suppressed Exception was too verbose.

Does this PR introduce any user-facing change?

Yes. Stacktraces of Exceptions coming from a LazyTry (e.g. QueryExecution plans) should be more readable.

How was this patch tested?

Unit tests were adapted.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the CORE label Nov 18, 2024
@juliuszsompolski juliuszsompolski changed the title [SPARK-50338] Make LazyTry exceptions less verbose [SPARK-50338][CORE] Make LazyTry exceptions less verbose Nov 18, 2024
@juliuszsompolski
Copy link
Contributor Author

@yaooqinn @cloud-fan
Hmm... So I see that it happens in a pattern:
The LazyTry of executedPlan triggers the LazyTry of sparkPlan triggers the LazyTry of optimizedPlan triggers the LazyTry of analyzedPlan...
So if you get an AnalysisError and then trigger the executedPlan, the stitched together stack trace of exception will be the full stack trace going down into where the error originally happened in the analyzer, stitched with the stack trace of where it got called now... but then each LazyTry layer adds it's own suppressed exception of the full stack trace of where it was called from where it originally happened.
I guess I could still do something to see that if an exception already has a OriginalTryStackTraceException I could compare if we are within the same recursive call, and don't add it again if so...

But I will have to think about it. I won't have time for it this week, let's not merge it until I have a chance to think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant