Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 17, 2014, 11:12 a.m.) Review request for mesos and Ben Mahler. Changes --- New version: now Group::data() returns FutureOptionstring instead of Futurestring to capture the NONODE case. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs (updated) - src/log/network.hpp fc85a57a38f89190fe246f16cd1fde3168d70613 src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca src/tests/group_tests.cpp 7ed98956181f167e16cd723c049738f1c217c73b src/zookeeper/group.hpp 16f9b7b390551402e3c1eddaf5657aa18766b47c src/zookeeper/group.cpp 58491c01052b68ddaee6af32f33192d5a1f20e58 Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
On Sept. 16, 2014, 11:20 a.m., Ben Mahler wrote: src/master/detector.cpp, lines 396-404 https://reviews.apache.org/r/25663/diff/1/?file=689921#file689921line396 Any way to explicitly ignore the no node case? Here we're ignoring all failures, which is a bit scary, since it's not obvious why the more serious errors would be caught elsewhere. I haven't put a lot of thought into this, could we return a FutureOptionstring data? Or should we consider wrapping our things in some kind of ZKOperationstring which lets us look at the various error cases? Happy to chat further! Yeah I was (unnecessarily) concerned about the ambiguity of returning a None if the method is changed to: ResultOptionstring GroupProcess::doData(const Group::Membership membership). Group needs to interpret the three different cases from the return value: 1. retryable error - retry 2. nonode - None 3. non-retryable error - Failure It turns out it's not that hard and we have precedence in the code base: https://github.com/apache/mesos/blob/190e87c51d25646fa501ffca0bf7150157982050/src/state/zookeeper.cpp#L427 So I revised the review to have Group return a None in the case of NONODE. - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/#review53558 --- On Sept. 17, 2014, 11:12 a.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 17, 2014, 11:12 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs - src/log/network.hpp fc85a57a38f89190fe246f16cd1fde3168d70613 src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca src/tests/group_tests.cpp 7ed98956181f167e16cd723c049738f1c217c73b src/zookeeper/group.hpp 16f9b7b390551402e3c1eddaf5657aa18766b47c src/zookeeper/group.cpp 58491c01052b68ddaee6af32f33192d5a1f20e58 Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/#review53719 --- Bad patch! Reviews applied: [25663] Failed command: ./support/mesos-style.py Error: Checking 506 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo src/log/network.hpp:142: Lines should be = 80 characters long [whitespace/line_length] [2] Total errors found: 1 - Mesos ReviewBot On Sept. 17, 2014, 6:12 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 17, 2014, 6:12 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs - src/log/network.hpp fc85a57a38f89190fe246f16cd1fde3168d70613 src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca src/tests/group_tests.cpp 7ed98956181f167e16cd723c049738f1c217c73b src/zookeeper/group.hpp 16f9b7b390551402e3c1eddaf5657aa18766b47c src/zookeeper/group.cpp 58491c01052b68ddaee6af32f33192d5a1f20e58 Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 17, 2014, 12:26 p.m.) Review request for mesos and Ben Mahler. Changes --- Minor styling fix. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs (updated) - src/log/network.hpp fc85a57a38f89190fe246f16cd1fde3168d70613 src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca src/tests/group_tests.cpp 7ed98956181f167e16cd723c049738f1c217c73b src/zookeeper/group.hpp 16f9b7b390551402e3c1eddaf5657aa18766b47c src/zookeeper/group.cpp 58491c01052b68ddaee6af32f33192d5a1f20e58 Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/#review53739 --- Patch looks great! Reviews applied: [25663] All tests passed. - Mesos ReviewBot On Sept. 17, 2014, 7:26 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 17, 2014, 7:26 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs - src/log/network.hpp fc85a57a38f89190fe246f16cd1fde3168d70613 src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca src/tests/group_tests.cpp 7ed98956181f167e16cd723c049738f1c217c73b src/zookeeper/group.hpp 16f9b7b390551402e3c1eddaf5657aa18766b47c src/zookeeper/group.cpp 58491c01052b68ddaee6af32f33192d5a1f20e58 Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/#review53762 --- Ship it! Thanks for adding the test! Too bad there is so much duplicated code between Group and ZooKeeperStorage, we already solved this problem in ZooKeeperStorage! ;) - Ben Mahler On Sept. 17, 2014, 7:26 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 17, 2014, 7:26 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs - src/log/network.hpp fc85a57a38f89190fe246f16cd1fde3168d70613 src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca src/tests/group_tests.cpp 7ed98956181f167e16cd723c049738f1c217c73b src/zookeeper/group.hpp 16f9b7b390551402e3c1eddaf5657aa18766b47c src/zookeeper/group.cpp 58491c01052b68ddaee6af32f33192d5a1f20e58 Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/#review53558 --- src/master/detector.cpp https://reviews.apache.org/r/25663/#comment93256 Any way to explicitly ignore the no node case? Here we're ignoring all failures, which is a bit scary, since it's not obvious why the more serious errors would be caught elsewhere. I haven't put a lot of thought into this, could we return a FutureOptionstring data? Or should we consider wrapping our things in some kind of ZKOperationstring which lets us look at the various error cases? Happy to chat further! - Ben Mahler On Sept. 15, 2014, 9:24 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 15, 2014, 9:24 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs - src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs - src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/#review53429 --- Patch looks great! Reviews applied: [25663] All tests passed. - Mesos ReviewBot On Sept. 15, 2014, 9:24 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- (Updated Sept. 15, 2014, 9:24 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-1392 https://issues.apache.org/jira/browse/MESOS-1392 Repository: mesos-git Description --- See summary. Diffs - src/master/detector.cpp 6436b8ee7e1ab6451a6b999a1cfbb2f79190e6ca Diff: https://reviews.apache.org/r/25663/diff/ Testing --- make check. Thanks, Jiang Yan Xu
