You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2014/04/22 20:49:32 UTC

Review Request 20561: Added common/constants.hpp for constants shared by Master and Slave.

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

Review request for mesos, Ben Mahler and Vinod Kone.


Repository: mesos-git


Description
-------

Currently both Master & Slave need to know SLAVE_REREGISTER_TIMEOUT.
Also Zookeeper timeout can be placed here.


Diffs
-----

  src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/slave/constants.hpp 6d1cfda97162f9f332e7f92461b281035587e7fa 
  src/slave/constants.cpp 9811bd39e352a5097eaadb3ae6c6e4f43bc9cd98 
  src/slave/slave.cpp b3c428508c77bf1ca9154f9c22b5d83c27bc9d1e 
  src/tests/fault_tolerance_tests.cpp 4796149beb10a6c668762f5efecd86520b809c42 
  src/tests/slave_recovery_tests.cpp 72b6d42f0895f10fed041a5a1351e79b003e2d1b 

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


Testing
-------

make check.


Thanks,

Jiang Yan Xu


Re: Review Request 20561: Added common/constants.hpp for constants shared by Master and Slave.

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


Bad patch!

Reviews applied: [20561]

Failed command: git apply --index 20561.patch

Error:
 error: patch failed: src/slave/constants.hpp:44
error: src/slave/constants.hpp: patch does not apply
error: patch failed: src/slave/constants.cpp:30
error: src/slave/constants.cpp: patch does not apply
error: patch failed: src/slave/slave.cpp:732
error: src/slave/slave.cpp: patch does not apply


- Mesos ReviewBot


On April 22, 2014, 7 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20561/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 7 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently both Master & Slave need to know SLAVE_REREGISTER_TIMEOUT.
> Also Zookeeper timeout can be placed here.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/common/constants.hpp PRE-CREATION 
>   src/common/constants.cpp PRE-CREATION 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/slave/constants.hpp 6d1cfda97162f9f332e7f92461b281035587e7fa 
>   src/slave/constants.cpp 9811bd39e352a5097eaadb3ae6c6e4f43bc9cd98 
>   src/slave/slave.cpp b3c428508c77bf1ca9154f9c22b5d83c27bc9d1e 
>   src/tests/fault_tolerance_tests.cpp 4796149beb10a6c668762f5efecd86520b809c42 
>   src/tests/slave_recovery_tests.cpp 72b6d42f0895f10fed041a5a1351e79b003e2d1b 
> 
> Diff: https://reviews.apache.org/r/20561/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 20561: Added common/constants.hpp for constants shared by Master and Slave.

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

> On April 22, 2014, 12:05 p.m., Vinod Kone wrote:
> > Forgot to add common/constants.hpp ?
> > 
> > Also, thinking about it a bit more does SLAVE_REREGISTER_TIMEOUT have anything to do with the timeout used by slave observer? Currently, the timeout used during recovery is same as the one used by slave observer, while it needn't be. In my upcoming change I'll make the recovery timeout configurable. We can then bound the slave re-registration retries by some factor of that number (maybe half?).

That's my comment on https://reviews.apache.org/r/20502/ :)
Agreed. 

So this review can be put on hold I guess.


- Jiang Yan


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


On April 22, 2014, noon, Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20561/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, noon)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently both Master & Slave need to know SLAVE_REREGISTER_TIMEOUT.
> Also Zookeeper timeout can be placed here.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/common/constants.hpp PRE-CREATION 
>   src/common/constants.cpp PRE-CREATION 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/slave/constants.hpp 6d1cfda97162f9f332e7f92461b281035587e7fa 
>   src/slave/constants.cpp 9811bd39e352a5097eaadb3ae6c6e4f43bc9cd98 
>   src/slave/slave.cpp b3c428508c77bf1ca9154f9c22b5d83c27bc9d1e 
>   src/tests/fault_tolerance_tests.cpp 4796149beb10a6c668762f5efecd86520b809c42 
>   src/tests/slave_recovery_tests.cpp 72b6d42f0895f10fed041a5a1351e79b003e2d1b 
> 
> Diff: https://reviews.apache.org/r/20561/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 20561: Added common/constants.hpp for constants shared by Master and Slave.

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


Forgot to add common/constants.hpp ?

Also, thinking about it a bit more does SLAVE_REREGISTER_TIMEOUT have anything to do with the timeout used by slave observer? Currently, the timeout used during recovery is same as the one used by slave observer, while it needn't be. In my upcoming change I'll make the recovery timeout configurable. We can then bound the slave re-registration retries by some factor of that number (maybe half?).

- Vinod Kone


On April 22, 2014, 7 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20561/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 7 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently both Master & Slave need to know SLAVE_REREGISTER_TIMEOUT.
> Also Zookeeper timeout can be placed here.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/common/constants.hpp PRE-CREATION 
>   src/common/constants.cpp PRE-CREATION 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/slave/constants.hpp 6d1cfda97162f9f332e7f92461b281035587e7fa 
>   src/slave/constants.cpp 9811bd39e352a5097eaadb3ae6c6e4f43bc9cd98 
>   src/slave/slave.cpp b3c428508c77bf1ca9154f9c22b5d83c27bc9d1e 
>   src/tests/fault_tolerance_tests.cpp 4796149beb10a6c668762f5efecd86520b809c42 
>   src/tests/slave_recovery_tests.cpp 72b6d42f0895f10fed041a5a1351e79b003e2d1b 
> 
> Diff: https://reviews.apache.org/r/20561/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 20561: Added common/constants.hpp for constants shared by Master and Slave.

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


Can we discard this now?

- Vinod Kone


On April 22, 2014, 7 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20561/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 7 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Currently both Master & Slave need to know SLAVE_REREGISTER_TIMEOUT.
> Also Zookeeper timeout can be placed here.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
>   src/common/constants.hpp PRE-CREATION 
>   src/common/constants.cpp PRE-CREATION 
>   src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
>   src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
>   src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
>   src/slave/constants.hpp 6d1cfda97162f9f332e7f92461b281035587e7fa 
>   src/slave/constants.cpp 9811bd39e352a5097eaadb3ae6c6e4f43bc9cd98 
>   src/slave/slave.cpp b3c428508c77bf1ca9154f9c22b5d83c27bc9d1e 
>   src/tests/fault_tolerance_tests.cpp 4796149beb10a6c668762f5efecd86520b809c42 
>   src/tests/slave_recovery_tests.cpp 72b6d42f0895f10fed041a5a1351e79b003e2d1b 
> 
> Diff: https://reviews.apache.org/r/20561/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 20561: Added common/constants.hpp for constants shared by Master and Slave.

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

(Updated April 22, 2014, noon)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Forgot to add the new files.


Repository: mesos-git


Description
-------

Currently both Master & Slave need to know SLAVE_REREGISTER_TIMEOUT.
Also Zookeeper timeout can be placed here.


Diffs (updated)
-----

  src/Makefile.am a44ea42ec73067b8f58c729c0d0f6413fa5da01d 
  src/common/constants.hpp PRE-CREATION 
  src/common/constants.cpp PRE-CREATION 
  src/master/constants.hpp 27ae4f89cfd1ddb7db287d650af160a690f93c26 
  src/master/constants.cpp ed966bc5bcc4dbb0f96b966efe33f179723c6759 
  src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f 
  src/slave/constants.hpp 6d1cfda97162f9f332e7f92461b281035587e7fa 
  src/slave/constants.cpp 9811bd39e352a5097eaadb3ae6c6e4f43bc9cd98 
  src/slave/slave.cpp b3c428508c77bf1ca9154f9c22b5d83c27bc9d1e 
  src/tests/fault_tolerance_tests.cpp 4796149beb10a6c668762f5efecd86520b809c42 
  src/tests/slave_recovery_tests.cpp 72b6d42f0895f10fed041a5a1351e79b003e2d1b 

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


Testing
-------

make check.


Thanks,

Jiang Yan Xu