-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-26142][SQL] Implement shuffle read metrics in SQL #23128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,6 +82,14 @@ object SQLMetrics { | |
|
|
||
| private val baseForAvgMetric: Int = 10 | ||
|
|
||
| val REMOTE_BLOCKS_FETCHED = "remoteBlocksFetched" | ||
| val LOCAL_BLOCKS_FETCHED = "localBlocksFetched" | ||
| val REMOTE_BYTES_READ = "remoteBytesRead" | ||
| val REMOTE_BYTES_READ_TO_DISK = "remoteBytesReadToDisk" | ||
| val LOCAL_BYTES_READ = "localBytesRead" | ||
| val FETCH_WAIT_TIME = "fetchWaitTime" | ||
| val RECORDS_READ = "recordsRead" | ||
xuanyuanking marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Converts a double value to long value by multiplying a base integer, so we can store it in | ||
| * `SQLMetrics`. It only works for average metrics. When showing the metrics on UI, we restore | ||
|
|
@@ -194,4 +202,16 @@ object SQLMetrics { | |
| SparkListenerDriverAccumUpdates(executionId.toLong, metrics.map(m => m.id -> m.value))) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Create all shuffle read relative metrics and return the Map. | ||
| */ | ||
| def getShuffleReadMetrics(sc: SparkContext): Map[String, SQLMetric] = Map( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to name this create, rather than get, to imply we are creating a new set rather than just returning some existing sets.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, rename it to createShuffleReadMetrics and move to SQLShuffleMetricsReporter. Done in #23175. |
||
| REMOTE_BLOCKS_FETCHED -> createMetric(sc, "remote blocks fetched"), | ||
| LOCAL_BLOCKS_FETCHED -> createMetric(sc, "local blocks fetched"), | ||
| REMOTE_BYTES_READ -> createSizeMetric(sc, "remote bytes read"), | ||
| REMOTE_BYTES_READ_TO_DISK -> createSizeMetric(sc, "remote bytes read to disk"), | ||
| LOCAL_BYTES_READ -> createSizeMetric(sc, "local bytes read"), | ||
| FETCH_WAIT_TIME -> createTimingMetric(sc, "fetch wait time"), | ||
| RECORDS_READ -> createMetric(sc, "records read")) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.sql.execution.metric | ||
|
|
||
| import org.apache.spark.executor.TempShuffleReadMetrics | ||
|
|
||
| /** | ||
| * A shuffle metrics reporter for SQL exchange operators. | ||
| * @param tempMetrics [[TempShuffleReadMetrics]] created in TaskContext. | ||
| * @param metrics All metrics in current SparkPlan. This param should not empty and | ||
| * contains all shuffle metrics defined in [[SQLMetrics.getShuffleReadMetrics]]. | ||
| */ | ||
| private[spark] class SQLShuffleMetricsReporter( | ||
| tempMetrics: TempShuffleReadMetrics, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4 space indent
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks ,done in #23175. |
||
| metrics: Map[String, SQLMetric]) extends TempShuffleReadMetrics { | ||
| private[this] val _remoteBlocksFetched = metrics(SQLMetrics.REMOTE_BLOCKS_FETCHED) | ||
| private[this] val _localBlocksFetched = metrics(SQLMetrics.LOCAL_BLOCKS_FETCHED) | ||
| private[this] val _remoteBytesRead = metrics(SQLMetrics.REMOTE_BYTES_READ) | ||
| private[this] val _remoteBytesReadToDisk = metrics(SQLMetrics.REMOTE_BYTES_READ_TO_DISK) | ||
| private[this] val _localBytesRead = metrics(SQLMetrics.LOCAL_BYTES_READ) | ||
| private[this] val _fetchWaitTime = metrics(SQLMetrics.FETCH_WAIT_TIME) | ||
| private[this] val _recordsRead = metrics(SQLMetrics.RECORDS_READ) | ||
|
|
||
| override def incRemoteBlocksFetched(v: Long): Unit = { | ||
| _remoteBlocksFetched.add(v) | ||
| tempMetrics.incRemoteBlocksFetched(v) | ||
| } | ||
| override def incLocalBlocksFetched(v: Long): Unit = { | ||
| _localBlocksFetched.add(v) | ||
| tempMetrics.incLocalBlocksFetched(v) | ||
| } | ||
| override def incRemoteBytesRead(v: Long): Unit = { | ||
| _remoteBytesRead.add(v) | ||
| tempMetrics.incRemoteBytesRead(v) | ||
| } | ||
| override def incRemoteBytesReadToDisk(v: Long): Unit = { | ||
| _remoteBytesReadToDisk.add(v) | ||
| tempMetrics.incRemoteBytesReadToDisk(v) | ||
| } | ||
| override def incLocalBytesRead(v: Long): Unit = { | ||
| _localBytesRead.add(v) | ||
| tempMetrics.incLocalBytesRead(v) | ||
| } | ||
| override def incFetchWaitTime(v: Long): Unit = { | ||
| _fetchWaitTime.add(v) | ||
| tempMetrics.incFetchWaitTime(v) | ||
| } | ||
| override def incRecordsRead(v: Long): Unit = { | ||
| _recordsRead.add(v) | ||
| tempMetrics.incRecordsRead(v) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than putting this list and the getShuffleReadMetrics function here, we should move it into SQLShuffleMetricsReporter. Otherwise in the future when one adds another metric, he/she is likely to forget to update SQLShuffleMetricsReporter.