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 2013/12/08 21:09:41 UTC
Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Bugs: MESOS-868
https://issues.apache.org/jira/browse/MESOS-868
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
src/state/zookeeper.cpp 09b63d44e9349cab2d73659c939de3d8e96fbcc5
src/tests/zookeeper.hpp 1bc38c291cef39a4d255fd9065428a26d86248cb
src/tests/zookeeper.cpp 8bb49012d8dc46ef9f5a64ead1654253b9df8c21
src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
src/zookeeper/watcher.hpp 1db0386719c2a675d29b47b417dc856993062326
src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
Diff: https://reviews.apache.org/r/16111/diff/
Testing
-------
./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
Thanks,
Jiang Yan Xu
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Dec. 9, 2013, 8:14 p.m., Raul Gutierrez Segales wrote:
> > src/state/zookeeper.cpp, line 196
> > <https://reviews.apache.org/r/16111/diff/1/?file=395514#file395514line196>
> >
> > how is this method syncronized? i.e.; what happens if you (very quickly) get connected-> disconnected -> connected to a new server with a different timeout? Is it ensured that timeout will be set to the last negotiated timeout?
In our programming model these *Process classes have their methods called through a dispatching mechanism to a per object queue and within the object the execution of these methods are always serialized.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30036
-----------------------------------------------------------
On Dec. 10, 2013, 6:57 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 6:57 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30036
-----------------------------------------------------------
src/state/zookeeper.cpp
<https://reviews.apache.org/r/16111/#comment57543>
how is this method syncronized? i.e.; what happens if you (very quickly) get connected-> disconnected -> connected to a new server with a different timeout? Is it ensured that timeout will be set to the last negotiated timeout?
src/zookeeper/group.cpp
<https://reviews.apache.org/r/16111/#comment57544>
same question about syncronization?
- Raul Gutierrez Segales
On Dec. 9, 2013, 6:51 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 9, 2013, 6:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Raul Gutierrez Segales, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/state/zookeeper.cpp 09b63d44e9349cab2d73659c939de3d8e96fbcc5
> src/tests/zookeeper.hpp 1bc38c291cef39a4d255fd9065428a26d86248cb
> src/tests/zookeeper.cpp 8bb49012d8dc46ef9f5a64ead1654253b9df8c21
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/watcher.hpp 1db0386719c2a675d29b47b417dc856993062326
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30120
-----------------------------------------------------------
Ship it!
LGTM, but I'd like benh to take a look at state/zookeeper.{cpp|hpp}, just to confirm that no changes are needed there.
- Ben Mahler
On Dec. 10, 2013, 6:57 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 6:57 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30139
-----------------------------------------------------------
Ship it!
src/zookeeper/group.hpp
<https://reviews.apache.org/r/16111/#comment57707>
s/This/The/
src/zookeeper/group.cpp
<https://reviews.apache.org/r/16111/#comment57708>
timer = delay(zk->getSessionTimeout(),
self(),
&Self::timedout,
zk->getSessionId());
- Benjamin Hindman
On Dec. 10, 2013, 6:57 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 6:57 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use
the negotiated session timeout value as their local reconnect timeout.
Posted by lissa coffey <ed...@yahoo.com>.
> On Dec. 13, 2013, 12:09 a.m., Ben Mahler wrote:
> > This is now committed!
http://www.fixithere.net/ebay-customer-service/
- lissa
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30303
-----------------------------------------------------------
On Dec. 10, 2013, 10:37 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 10:37 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30303
-----------------------------------------------------------
This is now committed!
- Ben Mahler
On Dec. 10, 2013, 10:37 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 10:37 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/
-----------------------------------------------------------
(Updated Dec. 10, 2013, 10:37 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Changes
-------
BenH's
Bugs: MESOS-868
https://issues.apache.org/jira/browse/MESOS-868
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
Diff: https://reviews.apache.org/r/16111/diff/
Testing
-------
./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
Thanks,
Jiang Yan Xu
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Dec. 10, 2013, 7:08 p.m., Raul Gutierrez Segales wrote:
> > src/zookeeper/zookeeper.cpp, line 525
> > <https://reviews.apache.org/r/16111/diff/2/?file=396244#file396244line525>
> >
> > nit: (totally unfamiliar with this codebase so this might be off-base)
> >
> > is it worth it commenting that this needs to be called after ZOO_CONNECTED_STATE is reached? Otherwise you could get the requested session timeout as opposed to the actual negotiated one.
Thanks! I documented this in header.
/**
* \brief get the current session timeout.
*
* The session timeout requested by the client or the negotiated
* session timeout after the session is established with ZooKeeper.
*/
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30111
-----------------------------------------------------------
On Dec. 10, 2013, 6:57 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 6:57 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30111
-----------------------------------------------------------
src/zookeeper/zookeeper.cpp
<https://reviews.apache.org/r/16111/#comment57674>
nit: (totally unfamiliar with this codebase so this might be off-base)
is it worth it commenting that this needs to be called after ZOO_CONNECTED_STATE is reached? Otherwise you could get the requested session timeout as opposed to the actual negotiated one.
- Raul Gutierrez Segales
On Dec. 10, 2013, 6:57 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 6:57 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/
-----------------------------------------------------------
(Updated Dec. 10, 2013, 6:57 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Changes
-------
Now added a ZooKeeper::getSessionTimeout to avoid passing the negotiated timeout through lots of plumbing.
Bugs: MESOS-868
https://issues.apache.org/jira/browse/MESOS-868
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
Diff: https://reviews.apache.org/r/16111/diff/
Testing
-------
./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
Thanks,
Jiang Yan Xu
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Dec. 9, 2013, 10:46 p.m., Ben Mahler wrote:
> > src/tests/zookeeper.hpp, line 88
> > <https://reviews.apache.org/r/16111/diff/1/?file=395515#file395515line88>
> >
> > timeout is only applicable for session connected events, so maybe this makes more sense as an Option<Duration> with a comment that reflects when it is Some? (We could of course have different Events but let's keep this patch simple :)).
> >
> > The code that assumes it to be some can do the appropriate CHECK.
This is no longer applicable with the addition of zk->getSessionTimeout()
> On Dec. 9, 2013, 10:46 p.m., Ben Mahler wrote:
> > src/zookeeper/zookeeper.hpp, line 56
> > <https://reviews.apache.org/r/16111/diff/1/?file=395523#file395523line56>
> >
> > Maybe s/timeout/sessionTimeout/ here and elsewhere to be more clear.
Dropped because the timeout is now directly accessible through ZooKeeper::getSessionTimeout()
> On Dec. 9, 2013, 10:46 p.m., Ben Mahler wrote:
> > src/zookeeper/zookeeper.cpp, line 375
> > <https://reviews.apache.org/r/16111/diff/1/?file=395524#file395524line375>
> >
> > Is this cast necessary? Looks like we're widening.
Now use implicit casting
> On Dec. 9, 2013, 10:46 p.m., Ben Mahler wrote:
> > src/zookeeper/group.cpp, lines 291-292
> > <https://reviews.apache.org/r/16111/diff/1/?file=395521#file395521line291>
> >
> > This also results in us using the negotiated session timeout for the subsequent zookeeper connection in expired(), was that intended? If so, perhaps mention that here.
Now reverted this change and only use zk->getSessoinTimeout when setting up the local reconnect timer.
Future ZooKeeper re-instantiations will still use the timeout requested by the client.
> On Dec. 9, 2013, 10:46 p.m., Ben Mahler wrote:
> > src/tests/zookeeper_test_server.cpp, lines 94-102
> > <https://reviews.apache.org/r/16111/diff/1/?file=395518#file395518line94>
> >
> > Is the cast needed on these two?
Now use implicit casts.
> On Dec. 9, 2013, 10:46 p.m., Ben Mahler wrote:
> > src/tests/zookeeper_test_server.cpp, line 82
> > <https://reviews.apache.org/r/16111/diff/1/?file=395518#file395518line82>
> >
> > Perhaps a comment here as to how you knew that the int represented milliseconds in the ZK code?
Done.
> On Dec. 9, 2013, 10:46 p.m., Ben Mahler wrote:
> > src/state/zookeeper.cpp, line 214
> > <https://reviews.apache.org/r/16111/diff/1/?file=395514#file395514line214>
> >
> > Ditto about the subsequent ZK connection using this timeout as well, can you add a note if this was what you intended?
> >
> > Why not just say:
> >
> > // Update the session timeout to the negotiated value.
> >
> > Here and elsewhere.
This is no longer applicable now that the member variable stays 'const'.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30048
-----------------------------------------------------------
On Dec. 10, 2013, 6:57 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2013, 6:57 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30048
-----------------------------------------------------------
src/state/zookeeper.cpp
<https://reviews.apache.org/r/16111/#comment57576>
Ditto about the subsequent ZK connection using this timeout as well, can you add a note if this was what you intended?
Why not just say:
// Update the session timeout to the negotiated value.
Here and elsewhere.
src/tests/zookeeper.hpp
<https://reviews.apache.org/r/16111/#comment57577>
timeout is only applicable for session connected events, so maybe this makes more sense as an Option<Duration> with a comment that reflects when it is Some? (We could of course have different Events but let's keep this patch simple :)).
The code that assumes it to be some can do the appropriate CHECK.
src/tests/zookeeper_test_server.hpp
<https://reviews.apache.org/r/16111/#comment57575>
Should these be const?
src/tests/zookeeper_test_server.cpp
<https://reviews.apache.org/r/16111/#comment57557>
Perhaps a comment here as to how you knew that the int represented milliseconds in the ZK code?
src/tests/zookeeper_test_server.cpp
<https://reviews.apache.org/r/16111/#comment57559>
Is the cast needed on these two?
src/tests/zookeeper_tests.cpp
<https://reviews.apache.org/r/16111/#comment57574>
Thanks for the test!
src/zookeeper/group.cpp
<https://reviews.apache.org/r/16111/#comment57573>
This also results in us using the negotiated session timeout for the subsequent zookeeper connection in expired(), was that intended? If so, perhaps mention that here.
src/zookeeper/zookeeper.hpp
<https://reviews.apache.org/r/16111/#comment57570>
Maybe s/timeout/sessionTimeout/ here and elsewhere to be more clear.
src/zookeeper/zookeeper.cpp
<https://reviews.apache.org/r/16111/#comment57571>
Is this cast necessary? Looks like we're widening.
- Ben Mahler
On Dec. 9, 2013, 6:51 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 9, 2013, 6:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Raul Gutierrez Segales, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/state/zookeeper.cpp 09b63d44e9349cab2d73659c939de3d8e96fbcc5
> src/tests/zookeeper.hpp 1bc38c291cef39a4d255fd9065428a26d86248cb
> src/tests/zookeeper.cpp 8bb49012d8dc46ef9f5a64ead1654253b9df8c21
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/watcher.hpp 1db0386719c2a675d29b47b417dc856993062326
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/#review30064
-----------------------------------------------------------
src/zookeeper/group.hpp
<https://reviews.apache.org/r/16111/#comment57595>
Rather than passing timeout to all of these functions, let's just add a function to our ZooKeeper class.
- Benjamin Hindman
On Dec. 9, 2013, 6:51 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16111/
> -----------------------------------------------------------
>
> (Updated Dec. 9, 2013, 6:51 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Raul Gutierrez Segales, and Vinod Kone.
>
>
> Bugs: MESOS-868
> https://issues.apache.org/jira/browse/MESOS-868
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
> src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
> src/state/zookeeper.cpp 09b63d44e9349cab2d73659c939de3d8e96fbcc5
> src/tests/zookeeper.hpp 1bc38c291cef39a4d255fd9065428a26d86248cb
> src/tests/zookeeper.cpp 8bb49012d8dc46ef9f5a64ead1654253b9df8c21
> src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
> src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
> src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
> src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
> src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
> src/zookeeper/watcher.hpp 1db0386719c2a675d29b47b417dc856993062326
> src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
> src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
>
> Diff: https://reviews.apache.org/r/16111/diff/
>
>
> Testing
> -------
>
> ./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16111: Fixed the zookeeper client wrappers to use the
negotiated session timeout value as their local reconnect timeout.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16111/
-----------------------------------------------------------
(Updated Dec. 9, 2013, 6:51 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Raul Gutierrez Segales, and Vinod Kone.
Changes
-------
reviewer +rgs
Bugs: MESOS-868
https://issues.apache.org/jira/browse/MESOS-868
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/jvm/org/apache/zookeeper.hpp dac14565a66397f153cdc059859286f8ac555919
src/state/zookeeper.hpp 90b660737f52df426759877feb979b14ac4b6811
src/state/zookeeper.cpp 09b63d44e9349cab2d73659c939de3d8e96fbcc5
src/tests/zookeeper.hpp 1bc38c291cef39a4d255fd9065428a26d86248cb
src/tests/zookeeper.cpp 8bb49012d8dc46ef9f5a64ead1654253b9df8c21
src/tests/zookeeper_test_server.hpp 97a8524600fe3a57cf084c0dea8e99e9a056c504
src/tests/zookeeper_test_server.cpp dc53d6a182a861544d2f9e7fa873be4c8c402856
src/tests/zookeeper_tests.cpp a5fe9e18fbaa88ea56662dc1b2e3d51fb0b50822
src/zookeeper/group.hpp facfb1fe31eeeb042c0e2b94d739101911620cdf
src/zookeeper/group.cpp 5c92c5f89d441b2555d928772fa40573660e3e5a
src/zookeeper/watcher.hpp 1db0386719c2a675d29b47b417dc856993062326
src/zookeeper/zookeeper.hpp 72435432e433fc0162f8b88e2045efcc42793a3a
src/zookeeper/zookeeper.cpp cc8a7caeedb2c109d4952a6520cc98565adaa700
Diff: https://reviews.apache.org/r/16111/diff/
Testing
-------
./bin/mesos-tests.sh --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*:ZooKeeperStateTest* -j --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle
Thanks,
Jiang Yan Xu