tgravescs commented on a change in pull request #32526:
URL: https://github.com/apache/spark/pull/32526#discussion_r637966953
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -920,7 +931,7 @@ private[spark] class ExecutorAllocationManager(
val attempts = resourceProfileIdToStageAttempt.getOrElse(rp,
Set.empty).toSeq
// attempts is a Set, change to Seq so we keep all values
attempts.map { attempt =>
- stageAttemptToNumRunningTask.getOrElseUpdate(attempt, 0)
+ stageAttemptToNumRunningTask.getOrElse(attempt, 0)
Review comment:
I would like to clarify this, is this causing a leak? ie are you
getting some event late or out of order such that this added an entry in
stageAttemptToNumRunningTask but then its never removed? I think this change
is actually ok but would like to know if its needed. I would also like to know
when it happened to see if perhaps there is a race we can handle elsewhere in
the stage attempt being added.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
