venkata91 commented on a change in pull request #34122:
URL: https://github.com/apache/spark/pull/34122#discussion_r792329331
##########
File path: core/src/main/scala/org/apache/spark/Dependency.scala
##########
@@ -135,6 +144,7 @@ class ShuffleDependency[K: ClassTag, V: ClassTag, C:
ClassTag](
def shuffleMergeId: Int = _shuffleMergeId
def setMergerLocs(mergerLocs: Seq[BlockManagerId]): Unit = {
+ assert(shuffleMergeEnabled || shuffleMergeAllowed)
Review comment:
Makes sense. I don't think we need `shuffleMergeEnabled` now with
`shuffleMergeAllowed` and `mergerLocs.isEmpty` can achieve the same purpose.
Only thing is earlier, we explicitly set `shuffleMergeEnabled` to false if
`shuffleMergeFinalized` is set to true in the retry/reuse case.
Would it make sense to clear `mergerLocs` in that case when
`shuffleMergeAllowed` is set to false (case where the stage is determinate and
also `shuffleMergeFinalized`)?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
