Skip to content

Commit

Permalink
[SPARK-49898][CORE] Fix documentation and default for event log task …
Browse files Browse the repository at this point in the history
…metrics accumulator logging flag from SPARK-42204

### What changes were proposed in this pull request?

This PR corrects an unintentional default behavior change from apache#39763

That PR introduced a new configuration, `spark.eventLog.includeTaskMetricsAccumulators`, to provide an ability for users to disable the redundant logging of task metrics information via the Accumulables field in the Spark event log task end logs.

I made a mistake in updating that PR description and code from the original version: the description says that the intent is to not change out of the box behavior, but the actual flag default was the opposite.

This new PR corrects both the flag default and the flag description to reflect the original intent of not changing default behavior.

### Why are the changes needed?

Roll back an unintentional behavior change.

### Does this PR introduce _any_ user-facing change?

Yes, it rolls back an unintentional default behavior change.

### How was this patch tested?

Existing unit tests.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#48372 from JoshRosen/fix-event-log-accumulable-defaults.

Authored-by: Josh Rosen <joshrosen@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
  • Loading branch information
JoshRosen authored and HyukjinKwon committed Oct 8, 2024
1 parent 78135dc commit ef142c4
Showing 1 changed file with 7 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -273,15 +273,15 @@ package object config {

private[spark] val EVENT_LOG_INCLUDE_TASK_METRICS_ACCUMULATORS =
ConfigBuilder("spark.eventLog.includeTaskMetricsAccumulators")
.doc("Whether to include TaskMetrics' underlying accumulator values in the event log (as " +
"part of the Task/Stage/Job metrics' 'Accumulables' fields. This configuration defaults " +
"to false because the TaskMetrics values are already logged in the 'Task Metrics' " +
"fields (so the accumulator updates are redundant). This flag exists only as a " +
"backwards-compatibility escape hatch for applications that might rely on the old " +
"behavior. See SPARK-42204 for details.")
.doc("Whether to include TaskMetrics' underlying accumulator values in the event log " +
"(as part of the Task/Stage/Job metrics' 'Accumulables' fields. The TaskMetrics " +
"values are already logged in the 'Task Metrics' fields (so the accumulator updates " +
"are redundant). This flag defaults to true for behavioral backwards compatibility " +
"for applications that might rely on the redundant logging. " +
"See SPARK-42204 for details.")
.version("4.0.0")
.booleanConf
.createWithDefault(false)
.createWithDefault(true)

private[spark] val EVENT_LOG_OVERWRITE =
ConfigBuilder("spark.eventLog.overwrite")
Expand Down

0 comments on commit ef142c4

Please sign in to comment.