Skip to content

[SPARK-53126][SQL][CONNECT] Refactor SparkErrorUtils#stackTraceToString to handle null input and use it to replace ExceptionUtils#getStackTrace #51844

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

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Aug 5, 2025

What changes were proposed in this pull request?

This PR refactor SparkErrorUtils#stackTraceToString to handle null input and use it to replace ExceptionUtils#getStackTrace.

Why are the changes needed?

To improve Spark's error utility functions.

Does this PR introduce any user-facing change?

No behavior change.

How was this patch tested?

  • Pass Github Actions

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

No.

throwable.printStackTrace(writer)
writer.flush()
}
new String(out.toByteArray, UTF_8)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of ExceptionUtils#getStackTrace is as follows:

/**
     * Gets the stack trace from a Throwable as a String, including suppressed and cause exceptions.
     *
     * <p>
     * The result of this method vary by JDK version as this method
     * uses {@link Throwable#printStackTrace(java.io.PrintWriter)}.
     * </p>
     *
     * @param throwable  the {@link Throwable} to be examined, may be null
     * @return the stack trace as generated by the exception's
     * {@code printStackTrace(PrintWriter)} method, or an empty String if {@code null} input
     */
    public static String getStackTrace(final Throwable throwable) {
        if (throwable == null) {
            return StringUtils.EMPTY;
        }
        final StringWriter sw = new StringWriter();
        throwable.printStackTrace(new PrintWriter(sw, true));
        return sw.toString();
    }

@dongjoon-hyun Do you agree to replace them with the refactored stackTraceToString?

The difference, as I see it, lies in the encoding—stackTraceToString always uses UTF-8, while getStackTrace uses the default encoding.

If this is an issue, I can add a new getStackTrace method without modifying stackTraceToString.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for this PR, @LuciferYang . Thank you.

@LuciferYang LuciferYang marked this pull request as draft August 5, 2025 08:44
@LuciferYang LuciferYang changed the title Refactor SparkErrorUtils#stackTraceToString to handle null input and use it to replace ExceptionUtils#getStackTrace [SPARK-53126][SQL][CONNECT] Refactor SparkErrorUtils#stackTraceToString to handle null input and use it to replace ExceptionUtils#getStackTrace Aug 5, 2025
@LuciferYang
Copy link
Contributor Author

Thank you @dongjoon-hyun ~

@LuciferYang
Copy link
Contributor Author

A RocksDB crash occurred again in the latest commit. I have a suspicion that the increase in the number of tests for the current SQL module may have made it more prone to exceeding the two-hour time limit, potentially leading to a RocksDB JNI crash when the process was forcibly terminated.

image

When the test was terminated, sql-slow pipeline had been running for 2 hours and 2 seconds.

image

@LuciferYang LuciferYang marked this pull request as ready for review August 5, 2025 16:19
@LuciferYang
Copy link
Contributor Author

@LuciferYang
Copy link
Contributor Author

Merged into master for Apache Spark 4.1.0. Thanks @dongjoon-hyun

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

Successfully merging this pull request may close these issues.

2 participants