Skip to content

fix: report native shuffle read metrics - #5554

Open
peterxcli wants to merge 2 commits into
apache:mainfrom
peterxcli:fix/native-shuffle-read-metrics
Open

fix: report native shuffle read metrics#5554
peterxcli wants to merge 2 commits into
apache:mainfrom
peterxcli:fix/native-shuffle-read-metrics

Conversation

@peterxcli

@peterxcli peterxcli commented Aug 29, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Part of #3996.

Rationale for this change

The direct native shuffle-read path bypasses Spark's normal decode path, so its records-read metric was not updated and temporary shuffle-read metrics were not merged into task metrics. This left the Spark Stage and SQL UIs with missing or zero native shuffle-read values.

What changes are included in this PR?

  • Pass the existing SQL shuffle-read metrics callback through CometShuffleBlockIterator.
  • Update Spark's records-read metric after each successfully decoded native shuffle batch.
  • Merge temporary shuffle-read metrics when a direct native shuffle task completes.
  • Extend task-metrics regressions to require the direct native ShuffleScan path, assert SQL/Stages UI aggregates, and preserve metrics on failed attempts.

How are these changes tested?

  • cargo build
  • cargo fmt --all -- --check
  • ./mvnw test -Dtest=none -Dsuites='org.apache.spark.sql.comet.CometTaskMetricsSuite native shuffle read' -Dscalastyle.skip=true (2 tests)
  • verified with a built Comet JAR on a fresh Spark 4.1.2 standalone cluster with two workers:
    • executor logs confirmed ShuffleScanExec
    • the Stage UI showed Shuffle Read Size / Records: 2004.6 KiB / 500000
    • the SQL UI showed 500,000 records, 2004.6 KiB read, and 32 local blocks

@comphead

Copy link
Copy Markdown
Contributor

I'm on it today, thanks @peterxcli

@comphead comphead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @peterxcli would you mind to attach a screenshot on what is changed? SparkUI shows currently records from shuffle read stages, just wondering what part of metrics is addressed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants