> On Aug. 13, 2013, 11:19 p.m., Ben Mahler wrote: > > src/master/detector.hpp, lines 53-62 > > <https://reviews.apache.org/r/13087/diff/4/?file=340172#file340172line53> > > > > My main comment here would be that it would be nicer on the clients of > > MasterDetector if they do not have to implement the following protocol > > externally: > > > > detect(pid, false) -> Failure -> detect(None(), true) -> Option<pid> -> > > detect(pid, false) -> Option<pid> -> detect(pid, false) > > > > I think it would be cleaner to return a result to encode the error and > > signal that we want to ignore errors rather than using an additional bool > > argument for this. If this returns a Result there would be no difference > > across the calls. The example above would become: > > > > detect(pid) -> Result::error -> detect(pid) -> Result::some -> > > detect(pid) -> Result::some -> detect(pid) > > > > Passing the result each time instead of distinguishing calls due to > > Failures.
I took your advice and revised the detector API this way. The contender API `virtual process::Future<process::Future<Nothing> > contend(bool force)` is a bit different: the caller of the method should know about the error state of the contender because there is not a separate loop inside (as in the detector that does some background work which may cause error). So I killed the bool flag (now `virtual process::Future<process::Future<Nothing> > contend()`) and now all calls on the method ignore previous errors. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13087/#review25097 ----------------------------------------------------------- On Oct. 3, 2013, 5:54 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13087/ > ----------------------------------------------------------- > > (Updated Oct. 3, 2013, 5:54 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-496 > https://issues.apache.org/jira/browse/MESOS-496 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/scheduler.hpp cf3ecdaaf40fd878a80fe0b6f7e61a0997329cbd > src/Makefile.am ee336130ad93d8b524c841f75be36f00d4a2b147 > src/detector/detector.hpp b0e66888050c1987b7200cdbf21ebe5e2e55e6c0 > src/detector/detector.cpp 12deefa0b9df3f4946d80f500caaa5199b8ea28e > src/local/local.cpp e4b5ec5b3dfae6dd89746353d3caaa8d7e117840 > src/local/main.cpp 5995c538077dea301f231668b4b5f905e73e7b3b > src/master/contender.hpp PRE-CREATION > src/master/contender.cpp PRE-CREATION > src/master/detector.hpp PRE-CREATION > src/master/detector.cpp PRE-CREATION > src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 > src/master/main.cpp 19fcb9f09d8fc03a0719aada7d216b575eb3069b > src/master/master.hpp 0aeec7fc540d44c03c1171f31a7281a4b0055925 > src/master/master.cpp ce8365f082a5f96ef64e33e526cb5047dff52127 > src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f > src/sched/sched.cpp c399f2481259683a8e178abb3478307042292f23 > src/slave/http.cpp 62fbb37a1924062543bf9db4229704bdef91601d > src/slave/main.cpp 750a12766bde64059bfd4635ea077cbd43cb4301 > src/slave/slave.hpp 22fb74b71a0f52d9d67b92ecc286fa8d350e41a4 > src/slave/slave.cpp 0ad457647dbc7870d478de2eab3a744f3b8704c5 > src/tests/allocator_tests.cpp e5dc0d7d9d547046e2c410a860f312761c8af58b > src/tests/cluster.hpp f743bb3251af81fb9d8afd51de4df6efcf289bb9 > src/tests/fault_tolerance_tests.cpp > 10e52c401476eb8416361de49b8e4061bb7ac4f3 > src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f > src/tests/isolator_tests.cpp cd3b3000060b379ef10e38a2a98a2eebe69d90fc > src/tests/master_contender_detector_tests.cpp PRE-CREATION > src/tests/master_detector_tests.cpp > 2d140ba1a364a7af4d643951d6016ac17dd10526 > src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 > src/tests/mesos.hpp bc365fb17a25dbddaaffe0502c291f56a4b8f1a5 > src/tests/mesos.cpp 776cb0f13d10b4ae437fe9a3c97dc8b3481290af > src/tests/slave_recovery_tests.cpp 097d43c1380274c8d2235d5c20f91c730f079355 > src/tests/zookeeper_tests.cpp 16c5fb7daaf684ece5542b42dd9e9ec66f4a41ce > > Diff: https://reviews.apache.org/r/13087/diff/ > > > Testing > ------- > > make check 100 times > > > Thanks, > > Jiang Yan Xu > >
