Spark 4.1: Add per-task scanDuration metric by venkata91 · Pull Request #17562 · apache/iceberg

venkata91 · 2026-08-09T02:22:06Z

Summary

Closes Spark: No metric for time spent reading data in a scan task #17563.
Iceberg's only read timer is totalPlanningDuration, which covers driver-side planning. There is no metric for time spent actually reading, so scan throughput can't be derived from Iceberg metrics alone.

Changes

Add scanDuration: wall time a task spends in BaseReader.next(), covering split open and iteration. Accumulated in a finally, so a task that throws still reports what it read.
TaskScanDuration from BatchDataReader and RowDataReader, aggregated by ScanDuration, mirroring the TotalPlanningDuration pair.
Nanoseconds rather than the millis used by totalPlanningDuration, since a split can be read in well under a millisecond.
Aggregate renders as total (min, med, max), matching MetricUtils.stringValue, so a straggler task stays visible.

Testing done

TestSparkReadMetrics (5): V1 and V2 tables, delete path, multi-task aggregation, and the non-vectorized RowDataReader path.
TestScanDuration (5): rendering, empty and straggler cases, input not mutated, locale independence.
0 failures. spotlessCheck and checkstyle pass.

Notes

Scoped to spark/v4.1. Happy to backport to 4.0 and 3.5 in a follow-up.

anoopj

The code change looks good to me. We might be missing code coverage for the non-vectorized path (RowDataReader) because the Spark read follows the vectorized path?

venkata91 · 2026-08-10T19:03:24Z

The code change looks good to me. We might be missing code coverage for the non-vectorized path (RowDataReader) because the Spark read follows the vectorized path?

@anoopj Addressed by adding unit test. Also added couple of integration tests as well. Thanks!

venkata91 · 2026-08-14T03:10:20Z

Gentle bump!

venkata91 · 2026-08-19T00:56:04Z

@anoopj Gentle bump!!

anoopj

LGTM. Thanks. We should get a committer to review/merge this as the next step.

venkata91 · 2026-08-26T19:49:34Z

cc @aokolnychyi or @anuragmantri for review please. Thanks!

venkata91 · 2026-09-06T21:39:30Z

Gentle ping @rdblue !!

uros-b

Thank you for working on this @venkata91, and thank you @anoopj for reviewing the changes! The accumulation logic looks good (monotonic += in the finally, single-threaded per reader; ScanDuration.NAME / TaskScanDuration.name() pair up). Leaving here just a couple of inline comments that should be considered before merge (the Locale.ROOT thing is pretty important, PTAL).

Also, the PR description is a bit stale now, "V1, V2 and V3 tables. 3 tests" no longer matches (five tests now, no V3 format exercised), and "Timed per next() call, not per row" is inaccurate for RowDataReader, where each next() yields a single InternalRow. Please update both.

uros-b · 2026-09-09T08:02:27Z

+    } else if (totalNanos < TimeUnit.SECONDS.toNanos(1)) {
+      return TimeUnit.NANOSECONDS.toMillis(totalNanos) + " ms";
+    } else {
+      return String.format("%.1f s", totalNanos / (double) TimeUnit.SECONDS.toNanos(1));


Locale.ROOT - ScanDuration.aggregateTaskMetrics uses String.format("%.1f s", ...) without a locale, so it renders "2,5 s" on a de/fr/pt JVM. The rest of the main source tree passes Locale.ROOT for numeric formatting; please use String.format(Locale.ROOT, "%.1f s", ...).

Addressed it.

uros-b · 2026-09-09T08:02:29Z

+  }
+
+  @Override
+  public String aggregateTaskMetrics(long[] taskMetrics) {


aggregateTaskMetrics sums per-task nanos into a single scalar, which discards straggler-task skew. Spark's own scanTime reports total (min, med, max) and you already have the full per-task array in hand - worth considering matching that shape.

Addressed it.

venkata91 · 2026-09-10T16:26:37Z

Thanks @uros-b for the review and your suggestions. Addressed all the review comments.

venkata91 · 2026-09-18T19:28:22Z

@uros-b Gentle ping! Do you think we can merge this PR?

uros-b · 2026-09-22T10:39:19Z

@venkata91 Please ping an expert Iceberg committer for further review here ^^

venkata91 · 2026-09-23T21:34:29Z

@nastra @singhpk234 @manuzhang mind taking a look when you get a chance? This has been open a while and is ready for a committer review.

It adds a scanDuration read metric alongside the existing totalPlanningDuration, following the same pattern as #15104. Approved by @anoopj, and review feedback from @uros-b has been addressed (the aggregate now renders as total (min, med, max) to match MetricUtils.stringValue, so straggler tasks stay visible). CI is green.

#17820 builds on this with the write-side equivalent (writeDuration and writeCloseDuration).

venkata91 · 2026-10-13T15:52:21Z

Gentle ping, please review @nastra @singhpk234 @manuzhang !! Thanks!

venkata91 · 2026-10-25T03:10:22Z

Gentle ping, please review @nastra

nastra · 2026-10-26T12:10:03Z

+public class TestScanDuration {
+
+  @Test
+  public void testAggregateTaskMetrics() {


let's avoid using test prefix in newly added tests

Done in 51ae742. I also dropped the prefix from the two tests this PR adds to TestSparkReadMetrics, and made TestScanDuration package-private. Thanks for the review! I'll open the Spark 4.2 port once this merges.

nastra

LGTM with one comment that would be good to address. Also please make sure to port this to Spark 4.2

manuzhang · 2026-10-26T15:19:37Z

@@ -88,7 +89,9 @@ class BatchDataReader extends BaseBatchReader<FileScanTask>
  @Override
  public CustomTaskMetric[] currentMetricsValues() {
    return new CustomTaskMetric[] {
-      new TaskNumSplits(numSplits), new TaskNumDeletes(counter().get())
+      new TaskNumSplits(numSplits),


Since the no-op initMEtricsValues is inherited, metrics from a later reader will override that of a previous one in the same task. Could you please fix it in a follow-up PR? Same for RowDataReader.

Good catch. With the no-op inherited, a later reader in the same task starts from zero instead of picking up the earlier reader's value, so only the last reader's duration survives. I'll fix BatchDataReader and RowDataReader together in a follow-up and link it here.

Fixed in #18726. It turned out Spark 4.2 is not affected, since it no longer calls initMetricsValues, so the fix is 4.1 only.

Iceberg reports scan bytes and file/manifest counts per task, but the only timer in the metrics system is totalPlanningDuration, which measures driver-side planning. There is no metric for how long a task spent actually reading data, so a scan's throughput cannot be derived from Iceberg metrics alone. Add scanDuration: wall time a task spends in BaseReader.next(), accumulated per call rather than per row so the timer costs nothing measurable relative to the read. Emitted as TaskScanDuration from BatchDataReader and RowDataReader, aggregated by ScanDuration, mirroring the existing TotalPlanningDuration pair. Values are nanoseconds: a single split can be read in well under a millisecond, so a millisecond timer would round most per-task reads to zero. ScanDuration formats the aggregate as ns/us/ms/s for the UI. Generated-by: Claude Opus 5

Generated-by: Claude Opus 5

venkata91 · 2026-11-01T06:18:33Z

@nastra I renamed the tests as you asked and rebased onto main, so CI is green now. Could you merge this when you get a chance? I'll open the 4.2 port right after.

github-actions Bot added the spark label Aug 9, 2026

venkata91 force-pushed the spark-task-scan-duration-metric branch 2 times, most recently from b88f53d to f1a6f78 Compare August 9, 2026 02:34

venkata91 mentioned this pull request Aug 9, 2026

Spark: No metric for time spent reading data in a scan task #17563

Open

1 of 3 tasks

anoopj reviewed Aug 10, 2026

View reviewed changes

anoopj approved these changes Aug 19, 2026

View reviewed changes

venkata91 mentioned this pull request Aug 26, 2026

Spark 4.1: Add per-task write duration metrics #17820

Open

uros-b reviewed Sep 9, 2026

View reviewed changes

nastra reviewed Oct 26, 2026

View reviewed changes

nastra approved these changes Oct 26, 2026

View reviewed changes

manuzhang reviewed Oct 26, 2026

View reviewed changes

venkata91 mentioned this pull request Nov 1, 2026

Spark 4.1: Keep read metrics from earlier readers in the same task #18726

Open

Venkata Krishnan Sowrirajan and others added 6 commits November 1, 2026 02:48

Spark 4.1: Cover RowDataReader in scanDuration test

042dab4

Spark 4.1: Trim comments, add aggregate and formatting tests

7b18068

Spark 4.1: Use Locale.ROOT when formatting the scan duration aggregate

9ff40e2

Spark 4.1: Report scan duration as total (min, med, max)

69aed84

Spark 4.1: Drop test prefix from newly added scan duration tests

901bf2b

Generated-by: Claude Opus 5

venkata91 force-pushed the spark-task-scan-duration-metric branch from 51ae742 to 901bf2b Compare November 1, 2026 02:49

Conversation

venkata91 commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing done

Notes

Uh oh!

anoopj left a comment

Choose a reason for hiding this comment

Uh oh!

venkata91 commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

venkata91 commented Aug 14, 2026

Uh oh!

venkata91 commented Aug 19, 2026

Uh oh!

anoopj left a comment

Choose a reason for hiding this comment

Uh oh!

venkata91 commented Aug 26, 2026

Uh oh!

venkata91 commented Sep 6, 2026

Uh oh!

uros-b left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkata91 commented Sep 10, 2026

Uh oh!

venkata91 commented Sep 18, 2026

Uh oh!

uros-b commented Sep 22, 2026

Uh oh!

venkata91 commented Sep 23, 2026

Uh oh!

venkata91 commented Oct 13, 2026

Uh oh!

venkata91 commented Oct 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nastra left a comment

Choose a reason for hiding this comment

Uh oh!

manuzhang Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkata91 commented Nov 1, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

venkata91 commented Aug 9, 2026 •

edited

Loading

venkata91 commented Aug 10, 2026 •

edited

Loading

manuzhang Oct 26, 2026 •

edited

Loading