[VL] Metrics: Include kPreloadSplitPrepareTimeNanos in kDataSourceAddSplitWallNanos

[zhztheplayer]

Breaking change on UI: Metric data source add split time total on Spark UI will be shown longer than before.

Include missing preloadSplitPrepareTimeNanos from the 3 split-adding metrics.

before:

after:

[rui-mo]

I recall that kWaitForPreloadSplitNanos was intentionally excluded. cc: @FelixYBW Not sure if this might impact any metric comparisons in your script.

[zhztheplayer]

By the way, kWaitForPreloadSplitNanos includes the duration of bloom-filter initialization, it's costly in current Gluten version.

[FelixYBW]

kDataSourceAddSplitWallNanos is used to breakdown the elapsed time of table scan. It should count the time waiting in main thread only.

kDataSourceAddSplitWallNanos count the time when there is no pending split in queue, we need to start a new split fetch.

kWaitForPreloadSplitNanos counts the time when a split is already prefetched but not returned yet.

kPreloadSplitPrepareTimeNanos counts the sum time of all outstanding prefetched splits time. It doesn't include the ondemand split fetch.

So here

      metrics_->get(Metrics::kDataSourceAddSplitWallNanos)[metricIndex] =
          runtimeMetric("sum", second->customStats, kDataSourceAddSplitWallNanos) +
          runtimeMetric("sum", second->customStats, kWaitForPreloadSplitNanos)

[FelixYBW]

@zhztheplayer can you fix?

[zhztheplayer]

@FelixYBW

The code you suggested is exactly the same with the current code, I'm holding this off until getting more time to test this

[FelixYBW]

No, remove the prepare time:

kDataSourceAddSplitWallNanos) +
runtimeMetric("sum", second->customStats, kWaitForPreloadSplitNanos) +
runtimeMetric("sum", second->customStats, kPreloadSplitPrepareTimeNanos);

[FelixYBW]

Oh, current upstream is correct. We needn't the PR.

[rui-mo]

Could you highlight the backward compatibility for metrics can be affected in the PR description?

[zhztheplayer]

@rui-mo I added a breaking change line in PR desc.

Waiting for @FelixYBW to confirm the background.

[FelixYBW]

hold on the merge. velox changed some metrics there recently. I need to take a look

[github-actions]

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[FelixYBW]

needn't fix