cloud-fan commented on code in PR #56597:
URL: https://github.com/apache/spark/pull/56597#discussion_r3456635771
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/metric/MetricsFailureInjectionSuite.scala:
##########
@@ -420,6 +420,58 @@ class MetricsFailureInjectionSuite
}
}
+ test("Three stage metrics block failure injection with AQE") {
+ // Same as the previous test but with AQE enabled. Under AQE each Exchange
is materialized
Review Comment:
This test drops four assertions its non-AQE sibling has —
`stage3Metric.value === 5` and the three `lastAttemptValueForDataset(finalDf)`
checks — yet the comment calls it the same test. If they don't hold under AQE
(e.g. dataset lookup through `AdaptiveSparkPlanExec`), say so here; otherwise
restore them so the AQE path gets equal coverage.
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/UpdateTableSuiteBase.scala:
##########
@@ -342,12 +342,12 @@ abstract class UpdateTableSuiteBase extends
RowLevelOperationSuiteBase {
}
test("metric values are stable across stage retries") {
- // Force a shuffle in the UPDATE plan via an IN-subquery (with broadcast
disabled), then
- // have the DAGScheduler corrupt the first attempt of every upstream
shuffle map stage.
- // Note: the current fetch-failure injection does not retry the writer
stage, so this
- // test passes equally well with plain SQLMetric — it only exercises the
SLAM-aware
- // read path. Follow-up #55738 will add infra to actually retry the writer
stage and
- // exercise the SLAM behavior end-to-end for UPDATE.
+ // INJECT_SHUFFLE_FETCH_FAILURES corrupts the partition-0 task of the
first successful
Review Comment:
Nothing here asserts a retry actually fired. With the writer single-counting
by design, the test passes even if the injection silently stops retrying — the
vacuous-pass gap this PR closes for metadata MERGE. Consider asserting an
upstream raw-metric overcount, or note that the new infra-level AQE test is
what guards retry-fires. (Same for the MERGE `if (!noMetadata)` path.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
