-
Notifications
You must be signed in to change notification settings - Fork 241
[AURON #2386] Resolve deserialized expression classes with an explicit class loader #2395
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?
Changes from all commits
03fab60
d9d3a63
85cb424
fa4047e
e2dc0b3
1f16a05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,32 @@ on: | |
| required: false | ||
| type: string | ||
| default: '' | ||
| assert-log-matches: | ||
| description: >- | ||
| Optional grep -E pattern that the TPC-DS run log must contain. Use it to assert that | ||
| the plan shape a job exists to cover was actually produced, so the job cannot pass | ||
| because the optimizer stopped generating it. Implies --print-plan. | ||
| required: false | ||
| type: string | ||
| default: '' | ||
| assert-log-not-matches: | ||
| description: >- | ||
| Optional grep -E pattern that must not appear in the TPC-DS run log. Spark retries | ||
| absorb task-level failures, so queries can report PASS while throwing hundreds of | ||
| exceptions; this asserts on the log instead. Keep the pattern specific to the defect | ||
| under test, otherwise unrelated failures are attributed to it. | ||
| required: false | ||
| type: string | ||
| default: '' | ||
| jar-on-system-classpath: | ||
| description: >- | ||
| Whether to copy the Auron jar into $SPARK_HOME/jars, where the application class | ||
| loader defines it. When false, Auron is defined by Spark's MutableURLClassLoader | ||
| instead, which is what spark-submit --jars and the application jar both feed. Some | ||
| class-loading defects only reproduce in the latter configuration. | ||
| required: false | ||
| type: string | ||
| default: 'true' | ||
| queries: | ||
| description: 'Optional list of queries to run' | ||
| required: false | ||
|
|
@@ -257,12 +283,18 @@ jobs: | |
| path: dev/tpcds_1g | ||
|
|
||
| - name: Install Auron JAR | ||
| env: | ||
| JAR_ON_SYSTEM_CLASSPATH: ${{ inputs.jar-on-system-classpath }} | ||
| run: | | ||
| ls -la | ||
| jar=$(ls -1 auron-${{ inputs.sparkver }}_${{ inputs.scalaver }}*.jar | head -n1) | ||
| [ -n "$jar" ] || { echo "No jar matched: auron-${{ inputs.sparkver }}_${{ inputs.scalaver }}*.jar"; exit 1; } | ||
| echo "AURON_SPARK_JAR=$jar" >> "$GITHUB_ENV" | ||
| cp "$jar" spark-bin-${{ inputs.sparkver }}_${{ inputs.scalaver }}/jars/ | ||
| if [ "$JAR_ON_SYSTEM_CLASSPATH" = "false" ]; then | ||
| echo "Auron stays off the system classpath (MutableURLClassLoader defines it)" | ||
| else | ||
| cp "$jar" spark-bin-${{ inputs.sparkver }}_${{ inputs.scalaver }}/jars/ | ||
| fi | ||
|
|
||
| - name: Setup Java and Maven cache | ||
| uses: actions/setup-java@v5 | ||
|
|
@@ -378,15 +410,65 @@ jobs: | |
| SPARK_VERSION: ${{ inputs.sparkver }} | ||
| SCALA_VERSION: ${{ inputs.scalaver }} | ||
| SPARK_HOME: spark-bin-${{ inputs.sparkver }}_${{ inputs.scalaver }} | ||
| PRINT_PLAN: ${{ inputs.assert-log-matches != '' && '--print-plan' || '' }} | ||
| run: | | ||
| ls -la | ||
| set -o pipefail | ||
| dev/auron-it/run-it.sh \ | ||
| ${{ inputs.extrasparkconf }} \ | ||
| --type tpcds \ | ||
| --data-location dev/tpcds_1g \ | ||
| --query-filter ${{ matrix.query }} \ | ||
| --result-check \ | ||
| --plan-check | ||
| $PRINT_PLAN \ | ||
| --plan-check 2>&1 | tee tpcds-run-${{ matrix.query }}.log | ||
|
|
||
| # The auron-it jar depends on the Auron uber jar and so bundles Auron classes itself, which | ||
| # means skipping the copy into $SPARK_HOME/jars is not on its own proof of how Auron was | ||
| # loaded. Assert the property the job actually depends on: that some loader other than the | ||
| # application class loader defined Auron. auron-it prints this at startup. | ||
| - name: Assert Auron is not on the system classpath | ||
| if: ${{ inputs.jar-on-system-classpath == 'false' }} | ||
| env: | ||
| QUERY_LOG: tpcds-run-${{ matrix.query }}.log | ||
| run: | | ||
| grep -E '^Auron (Class Loader|Code Source|On System Classpath):' "$QUERY_LOG" || true | ||
| if ! grep -qx 'Auron On System Classpath: false' "$QUERY_LOG"; then | ||
| echo "::error::Auron was defined by the application class loader, or auron-it did not report it" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Guards against the job silently testing nothing: --plan-check only compares golden plans | ||
| # on Spark 3.5 (see PlanStabilityChecker), so on other versions a job whose purpose is a | ||
| # particular plan shape would still pass if the optimizer stopped producing it. | ||
| - name: Assert the run log matches ${{ inputs.assert-log-matches }} | ||
| if: ${{ inputs.assert-log-matches != '' }} | ||
| env: | ||
| QUERY_LOG: tpcds-run-${{ matrix.query }}.log | ||
| PATTERN: ${{ inputs.assert-log-matches }} | ||
| run: | | ||
| count=$(grep -c -E "$PATTERN" "$QUERY_LOG" || true) | ||
| if [ "$count" -eq 0 ]; then | ||
| echo "::error::expected the run log to match '$PATTERN', found no occurrence" | ||
| exit 1 | ||
| fi | ||
| echo "Matched '$PATTERN' $count time(s)." | ||
|
|
||
| # Task-level failures are absorbed by Spark's task retries, so the queries can still | ||
| # report PASS while throwing hundreds of exceptions. Assert on the log directly. | ||
| - name: Assert the run log does not match ${{ inputs.assert-log-not-matches }} | ||
| if: ${{ inputs.assert-log-not-matches != '' }} | ||
| env: | ||
| QUERY_LOG: tpcds-run-${{ matrix.query }}.log | ||
| PATTERN: ${{ inputs.assert-log-not-matches }} | ||
| run: | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this run only the query or small query set known to reproduce the issue and match the specific deserialization failure signature? Running all 99 TPC-DS queries adds substantial CI cost, while grepping every ClassCastException can attribute unrelated failures to expression deserialization. I think something more focused might be better here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with both points. The job did not set queries, so it used the default matrix and ran all queries. Since q1, q2 and q3 are sufficient to reproduce this problem issue (the plan contains scalar subqueries that trigger the bug), so I changed it to I also made the pattern more specific. Before it matched any The fix is in the commit e2dc0b3. |
||
| count=$(grep -c -E "$PATTERN" "$QUERY_LOG" || true) | ||
| if [ "$count" -gt 0 ]; then | ||
| echo "::error::run log matched '$PATTERN' $count time(s)" | ||
| grep -m5 -E "$PATTERN" "$QUERY_LOG" || true | ||
| exit 1 | ||
| fi | ||
| echo "No occurrence of '$PATTERN'." | ||
|
|
||
| - name: Upload RSS log | ||
| if: ${{ failure() && (inputs.celebornver != '' || inputs.unifflever != '') }} | ||
|
|
||
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.
Could we add a positive check that the runtime bloom-filter ScalarSubquery was actually injected? Right now the job only checks that no ClassCastException occurred.
Also I think --plan-check is skipped for Spark 4.1, since PlanStabilityChecker currently supports only Spark 3.5. Without a positive assertion, the job could pass simply because the optimizer stopped producing the plan that triggers this path ..
Uh oh!
There was an error while loading. Please reload this page.
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.
You are right about
--plan-check.PlanStabilityCheckerreturns early for any version except spark-3.5. In my local run it just prints:So the job had no positive check at all. If the optimizer stops generating this plan, the job would still pass and we would not notice.
I added --print-plan to auron-it, and a new assert-log-matches input. For this job it is set to
might_contain, which is the runtime bloom filter probe. In my local q1–q3 run it appears 10 times, and now the job will fail if it disappears.This does not add extra cost.
QueryRunneralready builds the plan string every time, so the new flag only decides whether to print it.See commit e2dc0b3.