You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2012/09/26 20:01:32 UTC

Review Request: Refactored underlying ZooKeeper master detector into a libprocess process.

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

Review request for mesos, John Sirois, Vinod Kone, and Ben Mahler.


Description
-------

See summary (2 of 7).


Diffs
-----

  src/detector/detector.cpp 0246846 

Diff: https://reviews.apache.org/r/7290/diff/


Testing
-------

make check


Thanks,

Benjamin Hindman


Re: Review Request: Refactored underlying ZooKeeper master detector into a libprocess process.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7290/#review11996
-----------------------------------------------------------

Ship it!


Ship It!

- Ben Mahler


On Sept. 26, 2012, 6:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7290/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2012, 6:01 p.m.)
> 
> 
> Review request for mesos, John Sirois, Vinod Kone, and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary (2 of 7).
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 0246846 
> 
> Diff: https://reviews.apache.org/r/7290/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request: Refactored underlying ZooKeeper master detector into a libprocess process.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Sept. 26, 2012, 8:12 p.m., Vinod Kone wrote:
> > src/detector/detector.cpp, line 365
> > <https://reviews.apache.org/r/7290/diff/1/?file=160209#file160209line365>
> >
> >     While you are in this file, how about only creating nodes only if they don't exist? I remember there was a ticket assigned to you for the same?

Coming in subsequent review.


- Benjamin


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


On Sept. 26, 2012, 6:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7290/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2012, 6:01 p.m.)
> 
> 
> Review request for mesos, John Sirois, Vinod Kone, and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary (2 of 7).
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 0246846 
> 
> Diff: https://reviews.apache.org/r/7290/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request: Refactored underlying ZooKeeper master detector into a libprocess process.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7290/#review11937
-----------------------------------------------------------



src/detector/detector.cpp
<https://reviews.apache.org/r/7290/#comment25472>

    While you are in this file, how about only creating nodes only if they don't exist? I remember there was a ticket assigned to you for the same?



src/detector/detector.cpp
<https://reviews.apache.org/r/7290/#comment25473>

    I dont follow this function? Also please add the path to the log message and make it more explicit that this is an unexpected created event.



src/detector/detector.cpp
<https://reviews.apache.org/r/7290/#comment25474>

    ditto


- Vinod Kone


On Sept. 26, 2012, 6:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7290/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2012, 6:01 p.m.)
> 
> 
> Review request for mesos, John Sirois, Vinod Kone, and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary (2 of 7).
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 0246846 
> 
> Diff: https://reviews.apache.org/r/7290/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request: Refactored underlying ZooKeeper master detector into a libprocess process.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Sept. 26, 2012, 9:38 p.m., John Sirois wrote:
> > src/detector/detector.cpp, line 326
> > <https://reviews.apache.org/r/7290/diff/1/?file=160209#file160209line326>
> >
> >     Seconds(10) seems easier to read

Done.


> On Sept. 26, 2012, 9:38 p.m., John Sirois wrote:
> > src/detector/detector.cpp, line 446
> > <https://reviews.apache.org/r/7290/diff/1/?file=160209#file160209line446>
> >
> >     Seconds(10) - can you lift this to 1 spot and re-use?

Yes, done (with a comment to make this configurable).


- Benjamin


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


On Sept. 26, 2012, 6:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7290/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2012, 6:01 p.m.)
> 
> 
> Review request for mesos, John Sirois, Vinod Kone, and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary (2 of 7).
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 0246846 
> 
> Diff: https://reviews.apache.org/r/7290/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request: Refactored underlying ZooKeeper master detector into a libprocess process.

Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7290/#review11950
-----------------------------------------------------------

Ship it!



src/detector/detector.cpp
<https://reviews.apache.org/r/7290/#comment25485>

    Seconds(10) seems easier to read



src/detector/detector.cpp
<https://reviews.apache.org/r/7290/#comment25486>

    Seconds(10) - can you lift this to 1 spot and re-use?


- John Sirois


On Sept. 26, 2012, 6:01 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7290/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2012, 6:01 p.m.)
> 
> 
> Review request for mesos, John Sirois, Vinod Kone, and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary (2 of 7).
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 0246846 
> 
> Diff: https://reviews.apache.org/r/7290/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>