> On Oct. 24, 2014, 6:08 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 2301 > > <https://reviews.apache.org/r/26382/diff/7/?file=727958#file727958line2301> > > > > why is this TASK_ERROR instead of TASK_LOST? A relaunch would likely > > succeed here right? > > Dominic Hamon wrote: > I thought if the offer id is invalid it likely would be again next time. > Is that not the case?
no. they cannot reuse an offer in subsequent calls to launchTasks(). so the only possible way to relaunch the task would be to use a new offer. > On Oct. 24, 2014, 6:08 p.m., Vinod Kone wrote: > > src/common/http.cpp, line 103 > > <https://reviews.apache.org/r/26382/diff/7/?file=727941#file727941line103> > > > > what happens if this is not set? > > Dominic Hamon wrote: > nothing - i thought it was a useful thing to export in the JSON model > though. nothing, as in empty? or does it return a default source (enum 0)? if it's the latter it's a problem. you should guard this statement with status.has_source(). > On Oct. 24, 2014, 6:08 p.m., Vinod Kone wrote: > > include/mesos/mesos.proto, line 636 > > <https://reviews.apache.org/r/26382/diff/7/?file=727940#file727940line636> > > > > I think this will become a crutch. We should know the reasons for why a > > task is lost. i.e., kill this. > > Dominic Hamon wrote: > it still might be 'unknown' in cases where the caller doesn't set the > reason. is that acceptable? the caller is mesos code (master, slave) which should always know the reason. do you have an example? > On Oct. 24, 2014, 6:08 p.m., Vinod Kone wrote: > > include/mesos/mesos.proto, lines 624-629 > > <https://reviews.apache.org/r/26382/diff/7/?file=727940#file727940line624> > > > > Pull this one and Reason inside TaskStatus. > > > > Also kill the SOURCE_ and REASON_ prefixes. > > Dominic Hamon wrote: > and not enum TaskState? I was following that model. > > i'd like to keep the prefixes as it makes it easy to be sure in the > callsite that you have the right enum. Autocomplete then works nicely too. The name "TaskState" is fully qualified and it's easy to understand what it represents, whereas a top level "Reason" and "Source" doesn't tell us what it is referring to. Having SOURCE_ prefix inside a Source namespace (enum) seems a bit redundant. Callers already do set_*source(), so I don't think it is that ambigous. But it's upto you. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26382/#review58174 ----------------------------------------------------------- On Oct. 27, 2014, 5:18 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26382/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2014, 5:18 p.m.) > > > Review request for mesos, Vinod Kone and Bill Farner. > > > Bugs: MESOS-1830 and MESOS-343 > https://issues.apache.org/jira/browse/MESOS-1830 > https://issues.apache.org/jira/browse/MESOS-343 > > > Repository: mesos-git > > > Description > ------- > > Added source and reason, enabled TASK_ERROR, and made the changes necessary > throughout the codebase. > > > Diffs > ----- > > include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 > src/common/protobuf_utils.hpp 212d5124b9a4cc58e61719fa7f07a61cd166e834 > src/common/protobuf_utils.cpp a9b65e328c4c62bff7fbf5633dda25d742d79019 > src/examples/balloon_framework.cpp b05d5679fe2915142907af0b2dc00c6cd76eb9c1 > src/examples/java/TestFramework.java > bc593d0abfacb00690b1492b2b82c970f4e4de6d > src/examples/low_level_scheduler_libprocess.cpp > 7ef5ea78ade4ed856b97009fdfe31281f0a55c17 > src/examples/low_level_scheduler_pthread.cpp > 6e233a10117a1c7aa669806b5b430e746e227ee5 > src/examples/no_executor_framework.cpp > f98a0735b9f287e7f1bf98af6c2e9a47ca6a77b2 > src/examples/test_framework.cpp 187a611ebfe35cb13ee48aa5eca934cf55f34dea > src/master/master.cpp 9ebdc357fe679ed5bd54e0f4b9091ab9bcc2b00f > src/scaling/nested_exec.py 17e61706c75e6e849b0c40ae5232d8dc60804c55 > src/sched/sched.cpp 0fb8c7bda75545389f8024489b3c76ae115111f4 > src/slave/slave.cpp 5e7c107aa8f36aff954fa6a46c431a20c313ebc2 > src/tests/common/http_tests.cpp 912653b26615c86cc0204d80f58e6046c4b91a98 > src/tests/fault_tolerance_tests.cpp > a18a41a3e34ff112e04e693447d757403e5013bd > src/tests/master_authorization_tests.cpp > 652e80d0d4567b225c6ffb326725ddfde06f7fd3 > src/tests/master_tests.cpp f60e3761e5a558d379c56513937d2f59e898a490 > src/tests/reconciliation_tests.cpp 4ba53940951584d9baa2c7aa4e124814471206bc > src/tests/resource_offers_tests.cpp > fe66432b1bf75ee25feb73c4bb353e4d4e5b503f > > Diff: https://reviews.apache.org/r/26382/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
