> On 2012-04-04 00:37:57, Benjamin Hindman wrote: > > include/mesos/mesos.proto, line 91 > > <https://reviews.apache.org/r/4533/diff/2/?file=97830#file97830line91> > > > > Please update this comment to describe what timeout is used for.
Done. > On 2012-04-04 00:37:57, Benjamin Hindman wrote: > > include/mesos/mesos.proto, line 97 > > <https://reviews.apache.org/r/4533/diff/2/?file=97830#file97830line97> > > > > How about we use the default mechanism of protobufs, i.e.,: > > > > optional double timeout = 4 [default = 1.0]; > > > > Then you can kill the default in constants.hpp. ;) Done. > On 2012-04-04 00:37:57, Benjamin Hindman wrote: > > src/examples/test_framework.cpp, line 188 > > <https://reviews.apache.org/r/4533/diff/2/?file=97834#file97834line188> > > > > While I understand you're adding this to demonstrate how to use it, I > > think it's pretty self explanatory (especially after the comment gets added > > to mesos.proto) and I'd rather not have code in here that isn't being used > > (because future developers will assume that it's important for the running > > of the framework, which it is not). Ditto the rest of the examples. Done. > On 2012-04-04 00:37:57, Benjamin Hindman wrote: > > src/master/constants.hpp, line 57 > > <https://reviews.apache.org/r/4533/diff/2/?file=97835#file97835line57> > > > > While I like your intentions, I actually don't think there is a > > reasonable maximum. At Twitter, we actually set the timeout to several > > days! I think we should just eliminate this for now until we get a better > > idea of the right mechanism to build in to keep a framework for lasting > > forever (maybe the command line interface tool will be have a "destroy > > running framework" command). Done. > On 2012-04-04 00:37:57, Benjamin Hindman wrote: > > src/master/master.cpp, line 445 > > <https://reviews.apache.org/r/4533/diff/2/?file=97837#file97837line445> > > > > To make this more clear in the logs, how about: > > > > LOG(INFO) << "Giving framework " << framework->id << " " << timeout << > > " seconds to re-register"; Done. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4533/#review6668 ----------------------------------------------------------- On 2012-03-31 21:57:43, Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4533/ > ----------------------------------------------------------- > > (Updated 2012-03-31 21:57:43) > > > Review request for mesos and Benjamin Hindman. > > > Summary > ------- > > When I started this project, it was somewhat complicated, but with the recent > update that passes FrameworkInfo protobufs into the MesosSchedulerDriver, all > that was necessary to allow frameworks to set their own timeouts was to add > timeout as a field in FrameworkInfo and tell master.cpp to use this value. I > made it optional since some people might not care to set it and it would just > be an added complication, but I also added it to all three of the > test_frameworks (C++, Java, Python) to demonstrate its use. > > The choice for DEFAULT_FRAMEWORK_FAILOVER_TIMEOUT and > MAX_FRAMEWORK_FAILOVER_TIMEOUT was basically arbitrary, so I'm open to > suggestions on what might be more appropriate. > > > This addresses bug MESOS-143. > https://issues.apache.org/jira/browse/MESOS-143 > > > Diffs > ----- > > include/mesos/mesos.proto 75eee57 > include/mesos/scheduler.hpp dd09e3d > src/examples/java/TestFramework.java 4975760 > src/examples/python/test_framework.py dd9cf98 > src/examples/test_framework.cpp c408692 > src/master/constants.hpp 8248475 > src/master/master.hpp 8a34d7e > src/master/master.cpp 4dc9ee0 > > Diff: https://reviews.apache.org/r/4533/diff > > > Testing > ------- > > > Thanks, > > Thomas > >
