> On Oct. 28, 2013, 10:10 p.m., Niklas Nielsen wrote:
> > src/master/master.cpp, line 1398
> > <https://reviews.apache.org/r/14669/diff/9/?file=369195#file369195line1398>
> >
> > If we do that, we need to move the
> >
> > if (offerError.isSome()) {
> > ...
> > }
> >
> > block out as well and propagate the error with another:
> >
> > if (offerError.isSome()) {
> > break;
> > }
> >
> > Or else, an invalid offer won't send a TASK_LOST.
> > Do you prefer that approach?
>
> Vinod Kone wrote:
> I'm confused. There is already a if(oferError.isSome()) check on #1414.
> Would that not catch it?
Sorry for not being clear. What I mean is that we could do something like this
instead:
foreach (OfferID offerId, offerIds) {
Offer* offer = getOffer(offerId);
if (offer == NULL) {
offerError = OfferError::some(
"Offer " + stringify(offerId) + " is no longer valid");
break;
}
Slave* slave = getSlave(offer->slave_id());
foreach (OfferVisitor* visitor, offerVisitors) {
offerError = (*visitor)(
offer,
framework,
slave);
if (offerError.isSome()) {
break;
}
}
// Abort offer and task processing if any offer validation failed.
if (offerError.isSome()) {
break;
}
totalResources += offer->resources();
}
if (offerError.isSome()) {
LOG(WARNING) << "Failed to validate offer " << offerId
<< " : " << offerError.get();
foreach (const TaskInfo& task, tasks) {
const StatusUpdate& update = protobuf::createStatusUpdate(
framework->id,
task.slave_id(),
task.task_id(),
TASK_LOST,
"Task launched with invalid offers");
LOG(INFO) << "Sending status update " << update
<< " for launch task attempt on invalid offers: "
<< stringify(offerIds);
StatusUpdateMessage message;
message.mutable_update()->CopyFrom(update);
send(framework->pid, message);
}
}
Does that look better? Should achieve the same thing.
We need the last break, as the break in the foreach (OfferVisitor...) loop
won't break the outer loop.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14669/#review27626
-----------------------------------------------------------
On Oct. 28, 2013, 11:15 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14669/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2013, 11:15 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-749
> https://issues.apache.org/jira/browse/MESOS-749
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Running tasks on more than one offer belonging to a single slave can be
> useful in situations with multiple out-standing offers.
>
> This patch extends the usual launchTasks() to accept a vector of OfferIDs.
> The previous launchTasks (accepting a single OfferID) has been kept for
> backward compatibility, but this now calls the new launchTasks() with a
> one-element list.
> This also applied for the JNI and python interfaces, which accepts both
> formats as well.
>
> Offers are verified to belong to the same slave and framework, before
> resources are merged and used.
>
>
> Diffs
> -----
>
> include/mesos/scheduler.hpp fa1ffe8
> src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 9869929
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java ed4b4a3
> src/java/src/org/apache/mesos/SchedulerDriver.java 93aaa54
> src/master/master.hpp 1eba03f
> src/master/master.cpp 1147cc6
> src/messages/messages.proto a5dded2
> src/python/native/mesos_scheduler_driver_impl.cpp 059ed5d
> src/sched/sched.cpp 3049096
> src/tests/master_tests.cpp bf790d2
> src/tests/resource_offers_tests.cpp 2864c9a
>
> Diff: https://reviews.apache.org/r/14669/diff/
>
>
> Testing
> -------
>
> Three new tests has been added: LaunchCombinedOfferTest,
> LaunchAcrossSlavesTest and LaunchDuplicateOfferTest
> This test ensures that:
> 1) Multiple offers can be used to run a single task (requesting the sum of
> offer resources).
> 2) Offers cannot span multiple slaves.
> 3) No offers can appear more than once in offer list.
>
> $ make check
> ...
> [ RUN ] MasterTest.LaunchCombinedOfferTest
> [ OK ] MasterTest.LaunchCombinedOfferTest (2010 ms)
> [ RUN ] MasterTest.LaunchAcrossSlavesTest
> [ OK ] MasterTest.LaunchAcrossSlavesTest (3 ms)
> [ RUN ] MasterTest.LaunchDuplicateOfferTest
> [ OK ] MasterTest.LaunchDuplicateOfferTest (3 ms)
> ...
>
>
> Thanks,
>
> Niklas Nielsen
>
>
