> On Nov. 9, 2013, 2:03 a.m., Ben Mahler wrote: > > src/master/detector.cpp, line 102 > > <https://reviews.apache.org/r/13087/diff/12/?file=378516#file378516line102> > > > > Seems like a queue is more intuitive here?
It should be about the same I think. Just "inform every one and clean up after we are done" with no assumed order. Set has order but not FIFO order. I think for this use case both are fine. > On Nov. 9, 2013, 2:03 a.m., Ben Mahler wrote: > > src/master/detector.cpp, line 174 > > <https://reviews.apache.org/r/13087/diff/12/?file=378516#file378516line174> > > > > empty()? > > > > But why is this a WARNING? Won't we notify later when they call detect? Just because in normal StandaloneMasterDetector use cases the detect() is called before appoint(). I removed it as it's not necessary. > On Nov. 9, 2013, 2:03 a.m., Ben Mahler wrote: > > src/master/detector.cpp, lines 296-297 > > <https://reviews.apache.org/r/13087/diff/12/?file=378516#file378516line296> > > > > It seems like a discard / failure should propagate up to the caller > > instead. > > > > We can fail/discard the Promises futures instead of CHECKing, no? But I don't expect this to happen thus CHECKing. > On Nov. 9, 2013, 2:03 a.m., Ben Mahler wrote: > > src/master/detector.cpp, lines 310-317 > > <https://reviews.apache.org/r/13087/diff/12/?file=378516#file378516line310> > > > > We need to make sure the fetched() and the detected() cannot be > > re-ordered by enforcing order on the underlying group. > > > > The ordering problems in GroupProcess make me a little concerned here. > > We do not want to re-order detection results! (As in, we need to make sure > > fetched() runs before the subsequent detected())). > > > > A simple workaround here could be to do the subsequent detect() once > > we've fetched(): > > > > if (!_leader.get().isSome()) { > > ... > > > > // Continue detecting. > > detector.detect(_leader.get()) > > .onAny(defer(self(), &Self::detected, lambda::_1)); > > } else { > > // Fetch the data. > > group->data(_leader.get().get()) > > .onAny(defer(self(), &Self::fetched, lambda::_1)); > > } > > } > > > > Then inside fetched() { > > > > // Continue detecting. > > detector.detect(...) > > .onAny(...); > > } > > > > What do you think? I don't think this will be a problem: In normal cases the fetched() should be ahead of the next detected() but even if the order is reversed, it's just going to dispatch another fetch. So given that all fetches are correctly ordered by a queue, we should be fine. Right? > On Nov. 9, 2013, 2:03 a.m., Ben Mahler wrote: > > src/master/detector.cpp, line 162 > > <https://reviews.apache.org/r/13087/diff/12/?file=378516#file378516line162> > > > > Do you think discarded makes a little more sense? Yeah but I am mostly modeling these operations after Group, which fails pending promises in destructor. And since it is tightly integrated with Group so perhaps it's better to keep the semantics the same. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13087/#review28606 ----------------------------------------------------------- On Nov. 5, 2013, 10:29 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13087/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2013, 10:29 p.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 fa1ffe8b24a054556593d31eeae3fd1ac1761fb5 > src/Makefile.am 9780d07a23ca196c541a44a85499c2f44a574b9c > src/cli/resolve.cpp b05f5103ca898f74acb7dfe8f423ffc7e1872e09 > src/detector/detector.hpp 3aaebfe89f0b4bb5c64e7f4c8974d8b13bc6f6b1 > src/detector/detector.cpp 8d9f118757192ca38bcdb26663ed86effbcf3c9e > src/local/local.cpp 180756a929baed4dce97fdb8774f07010587468f > 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 45caf9dde0af5995dde73ae82e83e1d7169d17b8 > src/master/master.hpp e377af8b3ccd932ae411fa2df4c19642a7310d02 > src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 > src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b > src/sched/sched.cpp 042206853a68dd12c88801bd7bc6764b9b6b6c3f > src/slave/http.cpp 62fbb37a1924062543bf9db4229704bdef91601d > src/slave/main.cpp 750a12766bde64059bfd4635ea077cbd43cb4301 > src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb > src/slave/slave.cpp 90575db0ed9ce0953745cc23460fbcda240c7c0e > src/tests/allocator_tests.cpp b0beb72273e9d44a407d0e83e6c18a541b212089 > src/tests/authentication_tests.cpp 48a9323d03416ad9ee25fc19838d89678ff613bc > src/tests/cluster.hpp bea395a73b09df287fef743dae1aa836f4dac691 > src/tests/fault_tolerance_tests.cpp > d521f4be3e5297b7ab8f21f4173733bc2a2001c9 > src/tests/gc_tests.cpp 5459b78126d3607114ea171d3f76883f8355751c > src/tests/isolator_tests.cpp ab5b00aa7f12dbd299debc1a405ac50c59cec85c > src/tests/master_contender_detector_tests.cpp PRE-CREATION > src/tests/master_detector_tests.cpp > 06c586d29996cd599ae58dfd49cf1324aac0a6d6 > src/tests/master_tests.cpp bf790d24ff6b2fb199a04caebef523797b075e63 > src/tests/mesos.hpp ad39c095f9699fbe7958d52698fd2b0319e84eb9 > src/tests/mesos.cpp 351ffd614250847c52657b721a560eb2cf7c9443 > src/tests/slave_recovery_tests.cpp 37faf2054750080902cb06b62da9a9e2252c566f > > Diff: https://reviews.apache.org/r/13087/diff/ > > > Testing > ------- > > make check 100 times > > > Thanks, > > Jiang Yan Xu > >
