[jira] [Commented] (TEZ-911) Re-factor BroadcastShuffle related code to be more generic

["Hitesh Shah (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TEZ-911?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13922032#comment-13922032 ] 

Hitesh Shah commented on TEZ-911:
---------------------------------

Minor comments: 

{code}
-    if (numInputs != 0) {
+    if (numInputs != 0 && this.shuffleManager != null) {
       this.shuffleManager.shutdown();
     }
{code}
  - not sure why numInputs need to be checked

{code}
+  public static long getInitialMemoryReq(Configuration conf, long maxAvailableTaskMemory) {
     final float maxInMemCopyUse = conf.getFloat(
         TezJobConfig.TEZ_RUNTIME_SHUFFLE_INPUT_BUFFER_PERCENT,
         TezJobConfig.DEFAULT_TEZ_RUNTIME_SHUFFLE_INPUT_BUFFER_PERCENT);
{code}
Not sure why this is not being done directly in the configureAndStart function? 

Looking at ShuffledUnorderedKVInput, it is unclear as to the separation of initialization that happens in the initialize() function as compared to the start() function.

Otherwise, looks fine.




> Re-factor BroadcastShuffle related code to be more generic
> ----------------------------------------------------------
>
>                 Key: TEZ-911
>                 URL: https://issues.apache.org/jira/browse/TEZ-911
>             Project: Apache Tez
>          Issue Type: Task
>            Reporter: Siddharth Seth
>            Assignee: Siddharth Seth
>         Attachments: TEZ-911.1.norenames.txt, TEZ-911.1.renames.txt
>
>
> BroadcastShuffleManager should not be linked to Broadcast - it's really just responsible for moving data between the hosts. After that it's up to Broadcast components / others on how this is to be used.



--
This message was sent by Atlassian JIRA
(v6.2#6252)