You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2014/04/22 22:02:31 UTC

Review Request 20572: Integrated log storage into master.

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

Review request for mesos, Ben Mahler and Jiang Yan Xu.


Bugs: MESOS-1226
    https://issues.apache.org/jira/browse/MESOS-1226


Repository: mesos-git


Description
-------

See summary.


Diffs
-----

  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
  src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
  src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
  src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
  src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 

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


Testing
-------

Tests upcoming. I could either do it in this review or a subsequent one if others want to pitch in.


Thanks,

Vinod Kone


Re: Review Request 20572: Integrated log storage into master.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 23, 2014, midnight, Benjamin Hindman wrote:
> > src/master/flags.hpp, line 91
> > <https://reviews.apache.org/r/20572/diff/1/?file=564783#file564783line91>
> >
> >     I think you can call this just 'slave_reregister_timeout' because the only slave reregistration is after master failover, i.e., on master recovery, but the explicitness isn't so bad either (just verbose).

done.


> On April 23, 2014, midnight, Benjamin Hindman wrote:
> > src/master/main.cpp, line 178
> > <https://reviews.apache.org/r/20572/diff/1/?file=564784#file564784line178>
> >
> >     Let's not prefix it with '.' so that it isn't hidden from people in our filesystem. As for name, what about 'log_storage' to match the registry flag? Or 'registry_log_storage', or something similar?

called it "log_storage".


> On April 23, 2014, midnight, Benjamin Hindman wrote:
> > src/tests/cluster.hpp, line 298
> > <https://reviews.apache.org/r/20572/diff/1/?file=564787#file564787line298>
> >
> >     Hmm, weird scheme here ... I don't know why the initializer can't assume the path that we gave it was sufficient and it shouldn't have to append '.log'.

Not sure I understand. The Initializer needs the path to the replicated log.

s/.log/log_storage/ similar to what we do in main.cpp. Hopefully that clears up the confusion?


- Vinod


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


On April 22, 2014, 8:02 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 8:02 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Tests upcoming. I could either do it in this review or a subsequent one if others want to pitch in.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41091
-----------------------------------------------------------

Ship it!



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74500>

    I think you can call this just 'slave_reregister_timeout' because the only slave reregistration is after master failover, i.e., on master recovery, but the explicitness isn't so bad either (just verbose).



src/master/main.cpp
<https://reviews.apache.org/r/20572/#comment74496>

    Let's not prefix it with '.' so that it isn't hidden from people in our filesystem. As for name, what about 'log_storage' to match the registry flag? Or 'registry_log_storage', or something similar?



src/master/main.cpp
<https://reviews.apache.org/r/20572/#comment74498>

    We should make sure that 'zk' is not the empty string (or better make 'zk' be an Option).



src/tests/cluster.hpp
<https://reviews.apache.org/r/20572/#comment74497>

    Hmm, weird scheme here ... I don't know why the initializer can't assume the path that we gave it was sufficient and it shouldn't have to append '.log'.


- Benjamin Hindman


On April 22, 2014, 8:02 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 8:02 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Tests upcoming. I could either do it in this review or a subsequent one if others want to pitch in.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41205
-----------------------------------------------------------

Ship it!


I don't have additional comments. LGTM!

- Jie Yu


On April 23, 2014, 6:06 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 6:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 23, 2014, 7:17 p.m., Jiang Yan Xu wrote:
> > src/master/flags.hpp, line 98
> > <https://reviews.apache.org/r/20572/diff/1-2/?file=564783#file564783line98>
> >
> >     s/SLAVE_REREGISTER_TIEMOUT/SLAVE_REREGISTER_TIMEOUT
> >     
> >     Here and at the definition.

yikes. thanks.


> On April 23, 2014, 7:17 p.m., Jiang Yan Xu wrote:
> > src/master/main.cpp, lines 184-204
> > <https://reviews.apache.org/r/20572/diff/1-2/?file=564784#file564784line184>
> >
> >     Maybe a TODO or note here but we do support zk="file://..." for master election (through the same flag...)
> >     
> >     If the user wants a single master with zk="" (used to be unnecessary but legal) and log storage this will fail.
> >     
> >     If the user uses zk=file://... registry=log_stroage this fails too.
> >     
> >     The pain comes from reusing ZK (flag and znode path) for two things. Maybe we should have a followup cleanup for these argument incompatibility issues.

Good point. Added a TODO. 

regarding zk="", we only allowed it because it was not an optional flag. Now that "zk" is optional we should disallow it. We should fix that as part of the TODO (below) for MasterContender/Detector.


- Vinod


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


On April 23, 2014, 9:59 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 9:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41179
-----------------------------------------------------------



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74587>

    s/SLAVE_REREGISTER_TIEMOUT/SLAVE_REREGISTER_TIMEOUT
    
    Here and at the definition.



src/master/main.cpp
<https://reviews.apache.org/r/20572/#comment74613>

    Maybe a TODO or note here but we do support zk="file://..." for master election (through the same flag...)
    
    If the user wants a single master with zk="" (used to be unnecessary but legal) and log storage this will fail.
    
    If the user uses zk=file://... registry=log_stroage this fails too.
    
    The pain comes from reusing ZK (flag and znode path) for two things. Maybe we should have a followup cleanup for these argument incompatibility issues.


- Jiang Yan Xu


On April 23, 2014, 11:06 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 11:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 23, 2014, 6:44 p.m., Ben Mahler wrote:
> > src/local/local.cpp, line 129
> > <https://reviews.apache.org/r/20572/diff/2/?file=565397#file565397line129>
> >
> >     Maybe we should say "--work_dir" instead of "work directory"?

done. here and everywhere else.


> On April 23, 2014, 6:44 p.m., Ben Mahler wrote:
> > src/master/main.cpp, line 196
> > <https://reviews.apache.org/r/20572/diff/2/?file=565401#file565401line196>
> >
> >     Your comment in group.cpp says "replicas" but this one uses "log_replicas".

yea s/replicas/log_replicas/ everywhere. 'replicas' was a bit too generic for a znode name.


> On April 23, 2014, 6:44 p.m., Ben Mahler wrote:
> > src/tests/registrar_zookeeper_tests.cpp, lines 91-96
> > <https://reviews.apache.org/r/20572/diff/2/?file=565408#file565408line91>
> >
> >     Could we use createTask() from mesos.hpp?
> >     
> >     I see that's what we do in slave_recovery_tests.cpp.

i straight up copied this test from master_tests :) but sure i will use createTask() since its cleaner.


> On April 23, 2014, 6:44 p.m., Ben Mahler wrote:
> > src/zookeeper/group.cpp, line 709
> > <https://reviews.apache.org/r/20572/diff/2/?file=565410#file565410line709>
> >
> >     Might be nice to keep LOG(WARNING) but modify the check:
> >     
> >     if (sequence.isError() && tokens.back() != "replicas") {
> >     
> >     Otherwise we might not notice something dumping nodes in our path?

it seems a bit of abstraction leak for "group" to know about registrar's znodes. i'll add a TODO for now.


- Vinod


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


On April 23, 2014, 6:06 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 6:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

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



src/local/local.cpp
<https://reviews.apache.org/r/20572/#comment74582>

    Maybe we should say "--work_dir" instead of "work directory"?



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74586>

    How about:
    
    "It is imperative to set this value to include a majority of masters, i.e. Q > N/2 where N is the number of masters."
    
    Not sure whether we want to show C++ code instead of math.



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74588>

    "automatically initialize"
    
    Maybe a little blurb about the fact that if this is false, one has to manually initialize the log using the tool when running for the first time.



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74592>

    Technically we don't shut them down as they may already be shutdown, maybe we should say they will be removed from the registry and we will enforce that they be shut down if they re-communicate with the master?
    
    (Trying not to be too verbose but our documentation is lacking so I kind of like the idea of giving operators enough context through our flag descriptions).



src/master/main.cpp
<https://reviews.apache.org/r/20572/#comment74593>

    Right now the "strict" validation happens in the Master but we should be really careful when we remove that.
    
    Now I'm thinking it might be better to enforce that "in_memory" and "registry_strict" are not used together in main.cpp?
    
    How about a TODO on the Master::initialize code so that we know what to do when we remove the "registry_strict" check?



src/master/main.cpp
<https://reviews.apache.org/r/20572/#comment74595>

    Your comment in group.cpp says "replicas" but this one uses "log_replicas".



src/tests/cluster.hpp
<https://reviews.apache.org/r/20572/#comment74598>

    Ditto here: "replicas" used in group.cpp.



src/tests/registrar_zookeeper_tests.cpp
<https://reviews.apache.org/r/20572/#comment74601>

    Awesome, thanks for adding this!



src/tests/registrar_zookeeper_tests.cpp
<https://reviews.apache.org/r/20572/#comment74600>

    Could we use createTask() from mesos.hpp?
    
    I see that's what we do in slave_recovery_tests.cpp.



src/zookeeper/group.cpp
<https://reviews.apache.org/r/20572/#comment74596>

    Might be nice to keep LOG(WARNING) but modify the check:
    
    if (sequence.isError() && tokens.back() != "replicas") {
    
    Otherwise we might not notice something dumping nodes in our path?


- Ben Mahler


On April 23, 2014, 6:06 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 6:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41233
-----------------------------------------------------------

Ship it!


Ship It!

- Jiang Yan Xu


On April 23, 2014, 2:59 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 2:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 24, 2014, 3:17 p.m., Benjamin Hindman wrote:
> > src/zookeeper/group.cpp, lines 710-711
> > <https://reviews.apache.org/r/20572/diff/4/?file=566417#file566417line710>
> >
> >     While I like giving lots of information for posterity, I'm not sure the ZooKeeper group should know anything about the log replicas so a LOG(WARNING) seems unnecessary. Ignoring non-sequence nodes is just a general safety mechanism that we happen to be exploiting in Mesos. ;-)

i agree. see my reply to benm's comment above. maybe my TODO needed to be more explicit. anyway will kill it for now.


> On April 24, 2014, 3:17 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 267
> > <https://reviews.apache.org/r/20572/diff/4/?file=566409#file566409line267>
> >
> >     Any reason not to do this in this review? That way creating a master from 'cluster' and 'local' will also get this check. Seems like it's just an easy copy/paste IIUC.

i was just being lazy! fixed.


- Vinod


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


On April 24, 2014, 1:10 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 24, 2014, 1:10 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp c83292f10a488cf31c09d6917225704b77f3ee02 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41306
-----------------------------------------------------------

Ship it!



src/master/master.cpp
<https://reviews.apache.org/r/20572/#comment74733>

    Any reason not to do this in this review? That way creating a master from 'cluster' and 'local' will also get this check. Seems like it's just an easy copy/paste IIUC.



src/master/master.cpp
<https://reviews.apache.org/r/20572/#comment74728>

    s/atleast/at least/



src/tests/mesos.hpp
<https://reviews.apache.org/r/20572/#comment74730>

    s/MesoTest/MesosTest/



src/zookeeper/group.cpp
<https://reviews.apache.org/r/20572/#comment74732>

    While I like giving lots of information for posterity, I'm not sure the ZooKeeper group should know anything about the log replicas so a LOG(WARNING) seems unnecessary. Ignoring non-sequence nodes is just a general safety mechanism that we happen to be exploiting in Mesos. ;-)


- Benjamin Hindman


On April 24, 2014, 1:10 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 24, 2014, 1:10 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp c83292f10a488cf31c09d6917225704b77f3ee02 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41254
-----------------------------------------------------------

Ship it!



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74691>

    "atleast" one word? perhaps it's a valid spelling, i dunno...


- Jiang Yan Xu


On April 23, 2014, 6:10 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 6:10 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp c83292f10a488cf31c09d6917225704b77f3ee02 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

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

Ship it!


Thanks!


src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74779>

    trivial typo: s/atleast/at least/


- Ben Mahler


On April 24, 2014, 6:12 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 24, 2014, 6:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp c83292f10a488cf31c09d6917225704b77f3ee02 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

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

(Updated April 24, 2014, 6:12 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.


Changes
-------

benh's comments. NNFR.


Bugs: MESOS-1226
    https://issues.apache.org/jira/browse/MESOS-1226


Repository: mesos-git


Description
-------

See summary.


Diffs (updated)
-----

  src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
  src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/flags.hpp c83292f10a488cf31c09d6917225704b77f3ee02 
  src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
  src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
  src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
  src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
  src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
  src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
  src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
  src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 

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


Testing
-------

Added a new unit test that tests mesos cluster with registrar and zookeeper.

Also, updated external tests to use log storage but without zookeeper.

make check


Thanks,

Vinod Kone


Re: Review Request 20572: Integrated log storage into master.

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

(Updated April 24, 2014, 1:10 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.


Changes
-------

depends on.


Bugs: MESOS-1226
    https://issues.apache.org/jira/browse/MESOS-1226


Repository: mesos-git


Description
-------

See summary.


Diffs
-----

  src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
  src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/flags.hpp c83292f10a488cf31c09d6917225704b77f3ee02 
  src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
  src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
  src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
  src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
  src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
  src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
  src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
  src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 

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


Testing
-------

Added a new unit test that tests mesos cluster with registrar and zookeeper.

Also, updated external tests to use log storage but without zookeeper.

make check


Thanks,

Vinod Kone


Re: Review Request 20572: Integrated log storage into master.

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

(Updated April 24, 2014, 1:08 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.


Changes
-------

rebased.

also s/SLAVE_REREGISTER_TIMEOUT/MIN_SLAVE_REREGISTER_TIMEOUT/


Bugs: MESOS-1226
    https://issues.apache.org/jira/browse/MESOS-1226


Repository: mesos-git


Description
-------

See summary.


Diffs (updated)
-----

  src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
  src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/flags.hpp c83292f10a488cf31c09d6917225704b77f3ee02 
  src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
  src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
  src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
  src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
  src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
  src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
  src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
  src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 

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


Testing
-------

Added a new unit test that tests mesos cluster with registrar and zookeeper.

Also, updated external tests to use log storage but without zookeeper.

make check


Thanks,

Vinod Kone


Re: Review Request 20572: Integrated log storage into master.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41249
-----------------------------------------------------------


Bad patch!

Reviews applied: [20572]

Failed command: git apply --index 20572.patch

Error:
 error: patch failed: src/tests/mesos.hpp:230
error: src/tests/mesos.hpp: patch does not apply


- Mesos ReviewBot


On April 23, 2014, 9:59 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 9:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

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

(Updated April 23, 2014, 9:59 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.


Changes
-------

addressed comments.


Bugs: MESOS-1226
    https://issues.apache.org/jira/browse/MESOS-1226


Repository: mesos-git


Description
-------

See summary.


Diffs (updated)
-----

  src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
  src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
  src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
  src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
  src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
  src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
  src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
  src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
  src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
  src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 

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


Testing
-------

Added a new unit test that tests mesos cluster with registrar and zookeeper.

Also, updated external tests to use log storage but without zookeeper.

make check


Thanks,

Vinod Kone


Re: Review Request 20572: Integrated log storage into master.

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

(Updated April 23, 2014, 6:06 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.


Changes
-------

added "depends on".


Bugs: MESOS-1226
    https://issues.apache.org/jira/browse/MESOS-1226


Repository: mesos-git


Description
-------

See summary.


Diffs
-----

  src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
  src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
  src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
  src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
  src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
  src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
  src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
  src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
  src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
  src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 

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


Testing
-------

Added a new unit test that tests mesos cluster with registrar and zookeeper.

Also, updated external tests to use log storage but without zookeeper.

make check


Thanks,

Vinod Kone


Re: Review Request 20572: Integrated log storage into master.

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


PTAL.

- Vinod Kone


On April 23, 2014, 7:43 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 7:43 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

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

(Updated April 23, 2014, 7:43 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.


Changes
-------

Comments.

Added tests.


Bugs: MESOS-1226
    https://issues.apache.org/jira/browse/MESOS-1226


Repository: mesos-git


Description
-------

See summary.


Diffs (updated)
-----

  src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
  src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
  src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
  src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
  src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
  src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
  src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
  src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
  src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
  src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 

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


Testing (updated)
-------

Added a new unit test that tests mesos cluster with registrar and zookeeper.

Also, updated external tests to use log storage but without zookeeper.

make check


Thanks,

Vinod Kone


Re: Review Request 20572: Integrated log storage into master.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/flags.hpp, lines 92-94
> > <https://reviews.apache.org/r/20572/diff/1/?file=564783#file564783line92>
> >
> >     Using the word "recover(y)" in the description may help people understand the flag name "recovery_***"
> >     
> >     Maybe
> >     
> >     "The timeout within which all slaves are expected to re-register (be recovered) ..."?

removed "recovery_" prefix per benh's comment below.


> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/main.cpp, line 185
> > <https://reviews.apache.org/r/20572/diff/1/?file=564784#file564784line185>
> >
> >     alignment off by 1 space.

good catch!


> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/constants.hpp, lines 56-59
> > <https://reviews.apache.org/r/20572/diff/1/?file=564781#file564781line56>
> >
> >     I know it's the current way we do things but for the things we put in flags why do we put their defaults in a separate constants file instead of just defining them within flags.hpp?
> >     It'll be confusing whether to use the flag or the constants especially when the variables don't say "DEFAULT_***" Plus there is the non-POD global variable problem.

I think this is partly because of the gcc bug mentioned in constants.hpp. We would need a master/flags.cpp if we want to put these flag constants in master/flags.hpp. I will punt on this for now but will add a TODO.


> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/flags.hpp, line 90
> > <https://reviews.apache.org/r/20572/diff/1/?file=564783#file564783line90>
> >
> >     I guess slave backoff can't really use this because it doesn't handle "failover recovery" separately and still need to reregister within 75secs in case it's a network/ZK blip.

if it's a ZK blip only at the slave, the master wouldn't realize the slave disconnection. so the slave can always bound its re-registration retries on this value irrespective of whether the master failed over or not. does that make sense?


- Vinod


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


On April 22, 2014, 8:02 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 8:02 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Tests upcoming. I could either do it in this review or a subsequent one if others want to pitch in.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Jiang Yan Xu <ya...@jxu.me>.

> On April 22, 2014, 4:07 p.m., Jiang Yan Xu wrote:
> > src/master/flags.hpp, line 90
> > <https://reviews.apache.org/r/20572/diff/1/?file=564783#file564783line90>
> >
> >     I guess slave backoff can't really use this because it doesn't handle "failover recovery" separately and still need to reregister within 75secs in case it's a network/ZK blip.
> 
> Vinod Kone wrote:
>     if it's a ZK blip only at the slave, the master wouldn't realize the slave disconnection. so the slave can always bound its re-registration retries on this value irrespective of whether the master failed over or not. does that make sense?

If it's a full network blip and the slave fails to respond to pings the master is going to start the 75sec countdown. After network is restored and detected() invoked, the slave needs to rush to reregister within 75secs right?

It's probably too large to have a back off delay in the order of minutes no matter which case it is. Admittedly the large value has to be reached due to exponential increase from previous failures but these failures can be local and do not necessarily indicate an overloaded master.


- Jiang Yan


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


On April 23, 2014, 11:06 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 11:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Ben Mahler <be...@gmail.com>.

> On April 22, 2014, 11:07 p.m., Jiang Yan Xu wrote:
> > src/master/flags.hpp, line 90
> > <https://reviews.apache.org/r/20572/diff/1/?file=564783#file564783line90>
> >
> >     I guess slave backoff can't really use this because it doesn't handle "failover recovery" separately and still need to reregister within 75secs in case it's a network/ZK blip.
> 
> Vinod Kone wrote:
>     if it's a ZK blip only at the slave, the master wouldn't realize the slave disconnection. so the slave can always bound its re-registration retries on this value irrespective of whether the master failed over or not. does that make sense?
> 
> Jiang Yan Xu wrote:
>     If it's a full network blip and the slave fails to respond to pings the master is going to start the 75sec countdown. After network is restored and detected() invoked, the slave needs to rush to reregister within 75secs right?
>     
>     It's probably too large to have a back off delay in the order of minutes no matter which case it is. Admittedly the large value has to be reached due to exponential increase from previous failures but these failures can be local and do not necessarily indicate an overloaded master.

Yan, can the slave do anything with exited notifications here?


- Ben


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


On April 23, 2014, 9:59 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 9:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 364d63bb1f5dc8b63f72693eafd0b2feec231d13 
>   src/local/local.cpp 297f35b7755a688a95e58777f7846aa0ff3e247f 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/balloon_framework_test.sh f83240758b03871b8b53f45d0947c6171c9c3a93 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.hpp 7bc5e981a468b81f0460e2736c8d0b76518302de 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/tests/registrar_zookeeper_tests.cpp PRE-CREATION 
>   src/tests/script.cpp 09c7f3bfc8a4c3032116b90b44ca773deff4629d 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Added a new unit test that tests mesos cluster with registrar and zookeeper.
> 
> Also, updated external tests to use log storage but without zookeeper.
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 20572: Integrated log storage into master.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20572/#review41080
-----------------------------------------------------------

Ship it!



src/master/constants.hpp
<https://reviews.apache.org/r/20572/#comment74472>

    I know it's the current way we do things but for the things we put in flags why do we put their defaults in a separate constants file instead of just defining them within flags.hpp?
    It'll be confusing whether to use the flag or the constants especially when the variables don't say "DEFAULT_***" Plus there is the non-POD global variable problem.



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74482>

    I guess slave backoff can't really use this because it doesn't handle "failover recovery" separately and still need to reregister within 75secs in case it's a network/ZK blip.



src/master/flags.hpp
<https://reviews.apache.org/r/20572/#comment74483>

    Using the word "recover(y)" in the description may help people understand the flag name "recovery_***"
    
    Maybe
    
    "The timeout within which all slaves are expected to re-register (be recovered) ..."?



src/master/main.cpp
<https://reviews.apache.org/r/20572/#comment74464>

    kill the line?



src/master/main.cpp
<https://reviews.apache.org/r/20572/#comment74462>

    alignment off by 1 space.


- Jiang Yan Xu


On April 22, 2014, 1:02 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20572/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 1:02 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1226
>     https://issues.apache.org/jira/browse/MESOS-1226
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043 
>   src/master/main.cpp ec23781d2a1e687af031c060059de69079b179b4 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/state/log.hpp e25d1e5e1daf9a5a8cd6b7c6c9c95c38b58f892d 
>   src/tests/cluster.hpp 1862fe89a6c5897755133232d133dbf3664ed10a 
>   src/tests/mesos.cpp a9844e4cfef2eecbb30ca4bf1fa59d62edf93569 
>   src/zookeeper/group.cpp bdebc48e8ca793fa58cc0f9a0fc0daa5fb3a335e 
> 
> Diff: https://reviews.apache.org/r/20572/diff/
> 
> 
> Testing
> -------
> 
> Tests upcoming. I could either do it in this review or a subsequent one if others want to pitch in.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>