[jira] [Commented] (TEZ-879) Investigate synchronization in Inputs/Outputs .initialize() etc

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

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

Hitesh Shah commented on TEZ-879:
---------------------------------

Comments:

"String res = srcString.replaceAll("[^A-za-z0-9_]", "_");"
   - might be good to use a static Pattern.compile() 

{code}
+  }
+  
+    private void configureAndStart() throws IOException {
+      Preconditions.checkState(initialMemoryAvailable != -1,
+          "Initial Available memory must be configured before starting");
     merger.setInitialMemoryAvailable(initialMemoryAvailable);
     merger.configureAndStart();
   }
{code}
   - identation is off ? 

{code}
+    if (assignedMb <= 0) {
+      int initialMemRequestMb = 
+          this.conf.getInt(
+              TezJobConfig.TEZ_RUNTIME_IO_SORT_MB, 
+              TezJobConfig.DEFAULT_TEZ_RUNTIME_IO_SORT_MB);
+      LOG.warn("initialAvailableMemory: " + initialMemoryAvailable
+          + " is too low. Falling back to initial ask: " + initialMemRequestMb);
{code}
   - Seems wrong to do this. Why not throw an error and fail the task? 

{code}
-  private volatile long firstEventReceivedTime = -1;
+  private long firstEventReceivedTime = -1;
{code}
  - looks like we need to run findbugs on the codebase . 





 

> Investigate synchronization in Inputs/Outputs .initialize() etc
> ---------------------------------------------------------------
>
>                 Key: TEZ-879
>                 URL: https://issues.apache.org/jira/browse/TEZ-879
>             Project: Apache Tez
>          Issue Type: Task
>            Reporter: Siddharth Seth
>            Assignee: Siddharth Seth
>         Attachments: TEZ-879.1.txt
>
>
> Initialization, start etc happens in separate threads. Some synchronization - primarily for visibility - may be required.



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