xkrogen commented on a change in pull request #32389:
URL: https://github.com/apache/spark/pull/32389#discussion_r629498336
##########
File path:
core/src/test/scala/org/apache/spark/storage/ShuffleBlockFetcherIteratorSuite.scala
##########
@@ -123,6 +131,54 @@ class ShuffleBlockFetcherIteratorSuite extends
SparkFunSuite with PrivateMethodT
verify(wrappedInputStream.invokePrivate(delegateAccess()),
times(1)).close()
}
+ /**
+ * Get a blockByAddress iterator for a single BlockManagerId assuming all
blocks have the same
+ * size and `blockMapId`.
+ */
+ private def getBlocksByAddressForSingleBM(
Review comment:
Good idea! I focused on the single-BM case because it was simpler, but
you're right that there was still a lot of common logic to be reduced.
Actually, your suggestion made me realize that we only ever use this method
(and the new multi-BM method I created) to create an iterator which is then
passed to `getShuffleIteratorWithDefaults`, so I just made
`getShuffleIteratorWithDefaults` directly accept a `Map`. I think it's very
clean now, thank you for the suggestion!
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
