andygrove opened a new pull request, #7353:
URL: https://github.com/apache/datafusion-comet/pull/7353
## Which issue does this PR close?
Part of #6935. This covers the per-task metric and the docs. The
per-executor gauge for work that
plans hand to other Tokio tasks is left for a follow-up.
## Rationale for this change
Spark measures a task's CPU time as the CPU time of the task thread
(`executorCpuTime`). A native
block with no JVM input runs on Comet's Tokio workers while the task thread
waits for its batches,
so the usual scan stage (a native Parquet or Iceberg scan, through filters,
projections and a
partial aggregate, into the native shuffle writer) reports almost no CPU
time. Comparisons built
from `executorCpuTime` then overstate how much less CPU Comet uses than
Spark. Comet can't correct
`executorCpuTime` itself, because Spark sets it after `task.run` returns.
## What changes are included in this PR?
- `BatchProducer` reads the thread's CPU clock (`CLOCK_THREAD_CPUTIME_ID`)
before and after each
poll of the plan's stream and adds up the difference. A poll runs on one
thread from start to
finish, so this is the CPU time the poll used, whichever worker ran it,
and time spent waiting on
I/O is not counted. The total is published as `native_cpu_time` on the
root of the native block,
on the metrics update interval and when the plan is released. A plan with
a JVM input is polled
on the task thread, where `executorCpuTime` already counts it, so it
reports nothing.
- On the JVM side, `CometMetricNode.nativeCpuTimeMetrics` declares the
`native CPU time` SQL
metric. It is part of the baseline, join, sort-merge join, native scan and
shuffle metric sets,
and is declared directly on local limit, expand and the Iceberg native
scan, so every operator
that can be the root of a block has it. The native shuffle writer passes
it through. Native
writes are fed from the JVM, so they don't declare it; the native block
that feeds them reports
it on its own root. Spark leaves unset SQL metrics out of task-end events
and the SQL UI, so the
operators that aren't a block's root show nothing extra.
- As a SQL metric, its value is in each task's end event in the event log,
so event-log tools can
add it to `Executor CPU Time`.
- The metrics guide gets a "CPU Time" section. It describes the gap, the new
metric, and what
neither metric counts: work a block hands to other Tokio tasks or threads,
such as iceberg-rust's
delete loading, object store connection tasks, spill readers and blocking
file operations. For
that it points to container CPU time or Spark's `jvmCpuTime` gauge.
- `libc` is no longer optional in `native/core`. It was already in the
dependency graph.
Each poll now reads the clock twice. A read took about 100 ns on macOS. On
Linux, a thread CPU
clock read is a system call rather than a vDSO call; I haven't measured it
there.
## How are these changes tested?
- New Rust tests check that the clock counts only the calling thread's CPU
time, and that a batch
producer counts the CPU time its polls use but not the time its stream
waits between them.
- A new `CometTaskMetricsSuite` test, "native blocks without a JVM input
report native CPU time in
task-end events", covers two queries over a native scan:
- An aggregate with a native shuffle: the map tasks' task-end events
report the metric, and the
total equals the exchange's SQL metric. The result tasks, which read the
shuffle on the task
thread, report zero.
- A filter and projection: the block's root reports the metric.
The test fails when the native side doesn't publish the metric.
- `CometTaskMetricsSuite`, `CometExecSuite` and `CometAggregateSuite` pass
locally.
--
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]
