> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/README.txt, line 19
> > <https://reviews.apache.org/r/4768/diff/5/?file=105961#file105961line19>
> >
> >     I know it's obvious, but you might want to remind users that you'll 
> > need to install mpich2 on every machine in your cluster?.

Done.


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/README.txt, line 23
> > <https://reviews.apache.org/r/4768/diff/5/?file=105961#file105961line23>
> >
> >     Kill whitespace.

Done.


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/README.txt, line 25
> > <https://reviews.apache.org/r/4768/diff/5/?file=105961#file105961line25>
> >
> >     Kill whitespace.

Done.


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/nmpiexec.py, line 26
> > <https://reviews.apache.org/r/4768/diff/5/?file=105963#file105963line26>
> >
> >     s/mpd slots/mpd(s)

Done


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/nmpiexec.py, line 71
> > <https://reviews.apache.org/r/4768/diff/5/?file=105963#file105963line71>
> >
> >     If you move this check into the 'for offer in offers:' on line 60, then 
> > you'll only be doing the check and decline in one place (not also on lines 
> > 107 and 108).

Done


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/nmpiexec.py, line 118
> > <https://reviews.apache.org/r/4768/diff/5/?file=105963#file105963line118>
> >
> >     Again, I'm not sure how ifhn_slave is going to be used. Can you 
> > elaborate?

I left this in pending Jessica's response...it's removed now.


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/nmpiexec.py, line 121
> > <https://reviews.apache.org/r/4768/diff/5/?file=105963#file105963line121>
> >
> >     I love the long options! Thank you!


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/nmpiexec.py, line 209
> > <https://reviews.apache.org/r/4768/diff/5/?file=105963#file105963line209>
> >
> >     +1 to Jessica's comment.

This simplifies the trailing '/' check/fix to just os.path.join(options.path, 
""). 


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/nmpiexec.py, line 221
> > <https://reviews.apache.org/r/4768/diff/5/?file=105963#file105963line221>
> >
> >     +1 to Jessica's comment.

Unchanged after using the above.


> On 2012-05-04 01:41:20, Benjamin Hindman wrote:
> > frameworks/mpi/nmpiexec.py, line 230
> > <https://reviews.apache.org/r/4768/diff/5/?file=105963#file105963line230>
> >
> >     mpdtraceerr is not used, kill it please.

Done.


- Harvey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4768/#review7541
-----------------------------------------------------------


On 2012-05-02 13:29:50, Harvey Feng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4768/
> -----------------------------------------------------------
> 
> (Updated 2012-05-02 13:29:50)
> 
> 
> Review request for mesos, Benjamin Hindman, Charles Reiss, and Jessica.
> 
> 
> Summary
> -------
> 
> Some updates to point out:
> 
> -nmpiexec.py
>   -> 'mpdallexit' should terminate all slaves' mpds in the ring. I moved 
> 'driver.stop()' to statusUpdate() so that it stops when all tasks have been 
> finished, which occurs when the executor's launched mpd processes have all 
> exited. 
> -startmpd.py
>   -> Didn't remove cleanup(), and added code in shutdown() that manually 
> kills mpd processes. They might be useful during abnormal (cleanup) and 
> normal (shutdown) framework/executor termination...I think. cleanup() still 
> terminates all mpd's in the slave, but shutdown doesn't. 
>   -> killtask() stops the mpd associated with the given tid. 
>   -> Task states update nicely now. They correspond to the state of a task's 
> associated mpd process.
> -Readme
>   -> Included additional info on how to setup and run MPICH2 1.2 and nmpiexec 
> on OS X and Ubuntu/Linux
> 
> 
> This addresses bug MESOS-183.
>     https://issues.apache.org/jira/browse/MESOS-183
> 
> 
> Diffs
> -----
> 
>   frameworks/mpi/README.txt cdb4553 
>   frameworks/mpi/nmpiexec 517bdbc 
>   frameworks/mpi/nmpiexec.py a5db9c0 
>   frameworks/mpi/startmpd.py 8eeba5e 
>   frameworks/mpi/startmpd.sh 44faa05 
> 
> Diff: https://reviews.apache.org/r/4768/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Harvey
> 
>

Reply via email to