Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/20576#discussion_r171279296 --- Diff: core/src/main/scala/org/apache/spark/shuffle/sort/SortShuffleManager.scala --- @@ -188,9 +188,8 @@ private[spark] object SortShuffleManager extends Logging { log.debug(s"Can't use serialized shuffle for shuffle $shufId because the serializer, " + s"${dependency.serializer.getClass.getName}, does not support object relocation") false - } else if (dependency.aggregator.isDefined) { - log.debug( - s"Can't use serialized shuffle for shuffle $shufId because an aggregator is defined") + } else if (dependency.mapSideCombine) { + require(dependency.aggregator.isDefined, "Map-side combine without Aggregator specified!") --- End diff -- can we move this `require` to the constructor of `ShuffleDependency`? It appears many times in the codebase.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org