Implement getErrorClass explicitly to avoid AbstractMethodError on Spark builds without the deprecated default#197
Closed
dev-ankit wants to merge 2 commits into
Closed
Conversation
…ions
PulsarIllegalStateException and PulsarIllegalArgumentException implement
SparkThrowable but override only getCondition, relying on the deprecated
SparkThrowable.getErrorClass default (which delegates to getCondition).
Some Spark distributions — notably Databricks Runtime 18 (Spark 4.1) — leave
getErrorClass abstract rather than providing that default. When one of these
exceptions escapes a task, Spark's TaskResultGetter virtual-calls
getErrorClass(), finds no implementation, and raises:
java.lang.AbstractMethodError:
'java.lang.String org.apache.spark.SparkThrowable.getErrorClass()'
at org.apache.spark.sql.pulsar.PulsarIllegalStateException.getErrorClass
This kills the result-getter thread, so the original failure (e.g. an
incompatible-schema produce) is never reported and the Spark job hangs
indefinitely instead of failing.
Implement getErrorClass explicitly (delegating to the error class, same as
getCondition) so the body lives on the class itself. This is harmless on OSS
Spark, where it simply overrides the deprecated default, and restores correct
error propagation on distributions that drop it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@dev-ankit:Thanks for your contribution. For this PR, do we need to update docs? |
github-actions
Bot
added
doc-info-missing
|
@dev-ankit:Thanks for providing doc info! |
PulsarExceptions.pulsarSinkInvalidSchema references the error class "PULSAR_SINK_INVALID_SCHEMA", which is not present in error/pulsar-error-classes.json (only "PULSAR_SINK_INVALID_SCHEMA_TYPE" is), so constructing it raises INTERNAL_ERROR. Use pulsarProviderInvalidSaveMode, whose error class is defined, to exercise PulsarIllegalArgumentException. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
|
Superseded by #198 (re-opened from a personal fork instead of the grataio org fork). Closing this one. |
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
1 participant
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.
Motivation
PulsarIllegalStateExceptionandPulsarIllegalArgumentExceptionimplementSparkThrowablebut override onlygetCondition, relying on the deprecatedSparkThrowable.getErrorClassdefault (which delegates togetCondition).On OSS Spark 4.x that is fine —
getErrorClassis a@Deprecated defaultmethod See here. But some Spark distributions — notably Databricks Runtime 18 (Spark 4.1) — ship aSparkThrowablewheregetErrorClassis left abstract (no default body). When one of these connector exceptions escapes a task, Spark'sTaskResultGettervirtual-callsgetErrorClass(), finds no implementation, and throws:This kills the result-getter thread, so the original failure (e.g. a
PULSAR_SINK_INCOMPATIBLE_SCHEMAproduce error) is never reported and the Spark job hangs indefinitely instead of failing. On Databricks that meant multi-hour stuck jobs on idle-but-billed compute, with no error surfaced to the user.Modifications
getErrorClassexplicitly on bothPulsarIllegalStateExceptionandPulsarIllegalArgumentException, delegating to the error class (same value asgetCondition). The body then lives on the class itself instead of relying on the interface default. Harmless on OSS Spark (it overrides the deprecated default with the same value); restores correct error propagation on distributions that leavegetErrorClassabstract.PulsarExceptionsSuite.Verifying this change
Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
PulsarExceptionsSuiteasserts each exception declares its owngetErrorClass(viagetDeclaredMethod, which throws if the body falls back to the interface default) and returns the expected error class — it fails on the unpatched classes and passes with the fix. Validated locally with./mvnw -DskipTests test-compileagainst Spark 4.1.1.Documentation
Check the box below.
Need to update docs?
doc-requiredno-need-docdocNote: happy to file a linked tracking issue, or take a different approach (e.g. dropping
SparkThrowablefrom these two classes entirely) if you'd prefer.