You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "JoshRosen (via GitHub)" <gi...@apache.org> on 2024/07/04 02:27:16 UTC
[PR] [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs [spark]
JoshRosen opened a new pull request, #39763:
URL: https://github.com/apache/spark/pull/39763
### What changes were proposed in this pull request?
This PR modifies JsonProtocol in order to exclude certain redundant accumulator information from Spark event logs in order to save space and processing time.
Several event logs types contain both TaskMetrics and Accumulables, but there is redundancy in how the TaskMetrics data is stored:
- TaskMetrics are stored in a map called "Task Metrics" which maps from metric names to metric values.
- An "Accumulables" field contains information on accumulator updates from the task, but this field includes updates from the TaskMetrics internal accumulators (both the value from the task, plus a running "sum-so-far" from all of the tasks completed up to that point).
The redundant task metrics accumulables are not actually used by the Spark History Server: I verified this by reading AppStatusListener and SQLAppStatusListener.
I believe that this redundancy was introduced back in [SPARK-10620](https://issues.apache.org/jira/browse/SPARK-10620) when Spark 1.x's separate TaskMetrics implementation was replaced by the current accumulator-based version.
In this PR, I add logic to exclude TaskMetrics internal accumulators when writing this field.
Although I think it's highly unlikely that third-party non-Spark consumers of the event logs would be relying on this redundant information, I have added an "escape-hatch" configuration out of an abundance of caution. The new `spark.eventLog.includeTaskMetricsAccumulators` configuration (default `false`, meaning "exclude the redundant information") can be set to `true` to restore the old behavior.
### Why are the changes needed?
This change reduces the size of Spark event logs, especially for logs from applications that run many tasks. It should also have slight benefits on event log read and write speed (although I haven't tried to quantify this).
### Does this PR introduce _any_ user-facing change?
No user-facing changes in Spark History Server.
This could be considered a user-facing change from the perspective of third-party code which does its own direct processing of Spark event logs, hence the config. As I said, I think it's really unlikely that anyone is relying on this behavior, so I think it's okay to change the default behavior to one which benefits a majority of users (given that we have an escape-hatch fallback config).
### How was this patch tested?
New unit tests in `JsonProtocolSuite`.
Manual tests of event log size in `spark-shell` with a job that runs `spark.parallelize(1 to 1000, 1000).count()`. For this toy query, this PR's change shrunk the uncompressed event log size by ~15%. The relative size reduction will be even greater once other issues like https://issues.apache.org/jira/browse/SPARK-42206 or https://issues.apache.org/jira/browse/SPARK-42203 are fixed. The relative reduction will be smaller for tasks with many SQL metrics because those accumulables cannot be excluded.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] commented on pull request #39763: [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39763:
URL: https://github.com/apache/spark/pull/39763#issuecomment-1537580445
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-42204][CORE] Add option to disable redundant logging of TaskMetrics internal accumulators in event logs [spark]
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on PR #39763:
URL: https://github.com/apache/spark/pull/39763#issuecomment-2334915063
I pushed an empty commit to retest this. The only failing test is `org.apache.spark.storage.BlockManagerDecommissionIntegrationSuite` which was preexistingly flaky for reasons unrelated to this change.
Since I've been so delayed in actually merging this, I took another look to see if any intervening changes might have introduced logical conflicts but I didn't spot any potentially problematic changes.
Especially given that this is flagged-off-by-default opt-in functionality, I feel that this is a low-risk change.
Therefore I'm going to merge this now (finally!). In separate followups I may end up building on the configuration-plumbing infrastructure introduced here to add other opt-in size reduction optimizations.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #39763: [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #39763:
URL: https://github.com/apache/spark/pull/39763#discussion_r1089406188
##########
core/src/main/scala/org/apache/spark/util/JsonProtocol.scala:
##########
@@ -60,13 +71,19 @@ private[spark] object JsonProtocol {
private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
+ val defaultOptions: JsonProtocolOptions = new JsonProtocolOptions(new SparkConf(false))
+
/** ------------------------------------------------- *
* JSON serialization methods for SparkListenerEvents |
* -------------------------------------------------- */
def sparkEventToJsonString(event: SparkListenerEvent): String = {
Review Comment:
Add a note for this `sparkEventToJsonString` that it is only for tests ?
##########
core/src/main/scala/org/apache/spark/util/JsonProtocol.scala:
##########
@@ -60,13 +71,19 @@ private[spark] object JsonProtocol {
private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
+ val defaultOptions: JsonProtocolOptions = new JsonProtocolOptions(new SparkConf(false))
Review Comment:
`private[util]` ?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-42204][CORE] Add option to disable redundant logging of TaskMetrics internal accumulators in event logs [spark]
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen closed pull request #39763: [SPARK-42204][CORE] Add option to disable redundant logging of TaskMetrics internal accumulators in event logs
URL: https://github.com/apache/spark/pull/39763
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #39763: [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs
URL: https://github.com/apache/spark/pull/39763
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on a diff in pull request #39763: [WIP][CORE][SPARK-42204] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on code in PR #39763:
URL: https://github.com/apache/spark/pull/39763#discussion_r1088590148
##########
core/src/main/scala/org/apache/spark/util/JsonProtocol.scala:
##########
@@ -60,13 +71,19 @@ private[spark] object JsonProtocol {
private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
+ val defaultOptions: JsonProtocolOptions = new JsonProtocolOptions(new SparkConf(false))
+
/** ------------------------------------------------- *
* JSON serialization methods for SparkListenerEvents |
* -------------------------------------------------- */
def sparkEventToJsonString(event: SparkListenerEvent): String = {
Review Comment:
Method using default configurations is retained for the purposes of avoiding changes in a bunch of test code across several suites.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs [spark]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #39763:
URL: https://github.com/apache/spark/pull/39763#issuecomment-2209478200
We should remove the `[WIP]` before merging :-)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [WIP][SPARK-42204][CORE] Add option to disable redundant logging of TaskMetrics internal accumulators in event logs [spark]
Posted by "rednaxelafx (via GitHub)" <gi...@apache.org>.
rednaxelafx commented on PR #39763:
URL: https://github.com/apache/spark/pull/39763#issuecomment-2231799756
This looks good to me as well (non-binding). Thanks a lot for reviving this improvement!
Agreed that the historical Spark UI itself wouldn't be affected. The REST API will get some exposure to the change though. Hopefully there's no client out in the wild that relies on this redundant information.
The `JsonProtocolOptions` part looks pragmatic and I'm okay with that. If people are really annoyed by that, there's always the path of creating a `JsonProtocol` class that wraps the `JsonProtocolOptions` and delegates all operations down to the existing companion object. I don't think we have to do there here.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on a diff in pull request #39763: [WIP][CORE][SPARK-42204] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on code in PR #39763:
URL: https://github.com/apache/spark/pull/39763#discussion_r1088589702
##########
core/src/test/scala/org/apache/spark/util/JsonProtocolSuite.scala:
##########
@@ -778,12 +778,117 @@ class JsonProtocolSuite extends SparkFunSuite {
|}""".stripMargin
assert(JsonProtocol.sparkEventFromJson(unknownFieldsJson) === expected)
}
+
+ test("SPARK-42204: spark.eventLog.includeTaskMetricsAccumulators config") {
+ val includeConf = new JsonProtocolOptions(
+ new SparkConf().set(EVENT_LOG_INCLUDE_TASK_METRICS_ACCUMULATORS, true))
+ val excludeConf = new JsonProtocolOptions(
+ new SparkConf().set(EVENT_LOG_INCLUDE_TASK_METRICS_ACCUMULATORS, false))
+
+ val taskMetricsAccumulables = TaskMetrics
+ .empty
+ .nameToAccums
+ .filterKeys(!JsonProtocol.accumulableExcludeList.contains(_))
+ .values
+ .map(_.toInfo(Some(1), None))
+ .toSeq
+
+ val taskInfoWithTaskMetricsAccums = makeTaskInfo(222L, 333, 1, 333, 444L, false)
+ taskInfoWithTaskMetricsAccums.setAccumulables(taskMetricsAccumulables)
+ val taskInfoWithoutTaskMetricsAccums = makeTaskInfo(222L, 333, 1, 333, 444L, false)
+ taskInfoWithoutTaskMetricsAccums.setAccumulables(Seq.empty)
+
+ val stageInfoWithTaskMetricsAccums = makeStageInfo(100, 200, 300, 400L, 500L)
+ stageInfoWithTaskMetricsAccums.accumulables.clear()
+ stageInfoWithTaskMetricsAccums.accumulables ++= taskMetricsAccumulables.map(x => (x.id, x))
+ val stageInfoWithoutTaskMetricsAccums = makeStageInfo(100, 200, 300, 400L, 500L)
+ stageInfoWithoutTaskMetricsAccums.accumulables.clear()
+
+ // Test events which should be impacted by the config. Due to SPARK-42205 we need to test
Review Comment:
See https://issues.apache.org/jira/browse/SPARK-42205 for more details. Once that issue is fixed, we can drop the testcases for `TaskStart`, `JobStart`, and `StageSubmitted` here.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on a diff in pull request #39763: [WIP][CORE][SPARK-42204] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on code in PR #39763:
URL: https://github.com/apache/spark/pull/39763#discussion_r1088589402
##########
core/src/main/scala/org/apache/spark/util/JsonProtocol.scala:
##########
@@ -39,6 +40,16 @@ import org.apache.spark.scheduler.cluster.ExecutorInfo
import org.apache.spark.storage._
import org.apache.spark.util.Utils.weakIntern
+/**
+ * Helper class for passing configuration options to JsonProtocol.
+ * We use this instead of passing SparkConf directly because it lets us avoid
+ * repeated re-parsing of configuration values on each read.
+ */
+private[spark] class JsonProtocolOptions(conf: SparkConf) {
Review Comment:
This pattern is a bit weird, so I'd like to explain it in greater detail here:
To date, JsonProtocol has just been a big object with no configuration options of its own. All other event logging configurations take effect at higher levels, such as EventLoggingListener. We cannot implement this particular change at those levels, though: we shouldn't mutate the mutable TaskInfo or StageInfo objects referenced by the listener events and adding `copy()` methods to them would be expensive.
JsonProtocol is currently implemented as a global object, so there's no straightforward way to make it easily configurable and testable. If I did something like "get the active SparkContext's configuration and read the configuration value" on each call then this would destroy performance because configuration reading is not cheap.
One approach would be to refactor this so that JsonProtocol is no longer a singleton: we could give it a proper constructor which accepts a SparkConf. That's a much larger refactoring than I want to undertake right now, though, and it would involve significant updates in a bunch of test code.
The approach taken here is to thread through an `JsonProtocolOptions` class which holds the configuration values. Right now it only holds a single value, but this is done as future-proofing in case we add additional configurability. The lone caller in EventLoggingListener constructs the `JsonProtocolOptions` using SparkConf, so the configuration is only read from the conf once (avoiding conf reader performance overheads).
This is a more C-style approach than the usual OOP approach that we generally take, but I think it's okay in this limited internal context.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on pull request #39763: [WIP][CORE][SPARK-42204] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on PR #39763:
URL: https://github.com/apache/spark/pull/39763#issuecomment-1406021418
This is marked as "WIP" because I'm still deciding whether this merits a note in a migration guide / release note.
It's possible that I'm being overly cautious regarding backwards-compatibility for theoretical non-Spark consumers of these event logs, but I'm trying to avoid a situation where an impacted user has no straightforward mitigation.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] closed pull request #39763: [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #39763: [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs
URL: https://github.com/apache/spark/pull/39763
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on a diff in pull request #39763: [WIP][CORE][SPARK-42204] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on code in PR #39763:
URL: https://github.com/apache/spark/pull/39763#discussion_r1088589402
##########
core/src/main/scala/org/apache/spark/util/JsonProtocol.scala:
##########
@@ -39,6 +40,16 @@ import org.apache.spark.scheduler.cluster.ExecutorInfo
import org.apache.spark.storage._
import org.apache.spark.util.Utils.weakIntern
+/**
+ * Helper class for passing configuration options to JsonProtocol.
+ * We use this instead of passing SparkConf directly because it lets us avoid
+ * repeated re-parsing of configuration values on each read.
+ */
+private[spark] class JsonProtocolOptions(conf: SparkConf) {
Review Comment:
This pattern is a bit weird, so I'd like to explain it in greater length here:
To date, JsonProtocol has just been a big object with no configuration options of its own. All other event logging configurations take effect at higher levels, such as EventLoggingListener. We cannot implement this particular change at those levels, though: we shouldn't mutate the mutable TaskInfo or StageInfo objects referenced by the listener events and adding `copy()` methods to them would be expensive.
JsonProtocol is currently implemented as a global object, so there's no straightforward way to make it easily configurable and testable. If I did something like "get the active SparkContext's configuration and read the configuration value" on each call then this would destroy performance because configuration reading is not cheap.
One approach would be to refactor this so that JsonProtocol is no longer a singleton: we could give it a proper constructor which accepts a SparkConf. That's a much larger refactoring than I want to undertake right now, though, and it would involve significant updates in a bunch of test code.
The approach taken here is to thread through an `JsonProtocolOptions` class which holds the configuration values. Right now it only holds a single value, but this is done as future-proofing in case we add additional configurability. The lone caller in EventLoggingListener constructs the `JsonProtocolOptions` using SparkConf, so the configuration is only read from the conf once (avoiding conf reader performance overheads).
This is a more C-style approach than the usual OOP approach that we generally take, but I think it's okay in this limited internal context.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #39763: [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #39763:
URL: https://github.com/apache/spark/pull/39763#discussion_r1089370022
##########
core/src/main/scala/org/apache/spark/util/JsonProtocol.scala:
##########
@@ -39,6 +40,16 @@ import org.apache.spark.scheduler.cluster.ExecutorInfo
import org.apache.spark.storage._
import org.apache.spark.util.Utils.weakIntern
+/**
+ * Helper class for passing configuration options to JsonProtocol.
+ * We use this instead of passing SparkConf directly because it lets us avoid
+ * repeated re-parsing of configuration values on each read.
+ */
+private[spark] class JsonProtocolOptions(conf: SparkConf) {
Review Comment:
Sounds good to me.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #39763: [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #39763:
URL: https://github.com/apache/spark/pull/39763#issuecomment-1407050328
> This is marked as "WIP" because I'm still deciding whether this merits a note in a migration guide / release note.
> It's possible that I'm being overly cautious regarding backwards-compatibility for theoretical non-Spark consumers of these event logs, but I'm trying to avoid a situation where an impacted user has no straightforward short-term mitigation.
I have seen usages of this unfortunately - so having the flag would help.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [WIP][SPARK-42204][CORE] Remove redundant logging of TaskMetrics internal accumulators in event logs [spark]
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on PR #39763:
URL: https://github.com/apache/spark/pull/39763#issuecomment-2209509526
> We should remove the `[WIP]` before merging :-)
I will; since it's been a while since I looked at this code, I want to do another self-review and another round of manual testing before I merge.
I'm considering whether we might want to change this default for Spark 4.0; if so, I would propose to do that in a separate followup PR to also update docs + migration guide.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org