abstractdog commented on code in PR #473:
URL: https://github.com/apache/tez/pull/473#discussion_r3504650312
##########
tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java:
##########
@@ -180,7 +180,7 @@ public FetcherOrderedGrouped(HttpConnectionParams
httpConnectionParams,
@VisibleForTesting
protected void fetchNext() throws InterruptedException, IOException {
try {
- if (localDiskFetchEnabled && mapHost.getHost().equals(localShuffleHost)
&& mapHost.getPort() == localShufflePort) {
Review Comment:
regarding default `mapreduce_shuffle`: I don't know the exact history, but
we tend to not consider some pieces of our code production ready, even if
they're already tested on large scales: the Tez ShuffleHandler could be just
one example of this
I believe most of the Tez users use Tez for actual DAG execution and keep
using the mapreduce shuffle, which is battle-tested, and people are not aware
of the Tez ShuffleHandler improvements, so not motivated to use it
regarding packaging: yeah, given that the Tez ShuffleHandler jar has to be
installed separately for the NodeManager, it's not supposed to be part of the
tez.tar.gz, which is meant for the DAG execution
I believe we might want to keep the tar contents as is, but somehow deploy
the shuffle handler jar to the image separately
UPDATE: one more reference here: https://www.youtube.com/watch?v=0SazMDtIUgM
<- one of my favorite sessions about the tremendous work @jteagles ,
@kuhushukla, and the team put into the shuffle handler! 💪
--
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]
