You are right. I totally misread what is returned there. Sorry for the fuss! :-)
On So, 2014-09-14 at 16:48 -0700, Bill Farner wrote: > > > > assumes the latter > > > I think i see what you're saying, but i believe this is a case of > performing unnecessary work rather than unnecessarily evicting tasks. > > Consider the case you describe, where there are tasks on a host and a > pending task that can preempt them, but due to constraints may not run on > the host. We iterate through the possible victim tasks [1], however each > time the vetos [sic] List is non-empty [2]. As a result, we exit the loop > and return Optional.absent() [3], thus not actually flagging anything to > evict. > > I suppose what's confusing here is the variable name toPreemptTasks, which > suggests that we *are* going to preempt these tasks. However, these are > actually preemption candidates, and the pending task's constraints still > must be satisfied. > > I do agree to something you alluded — if Veto differentiated between a > constraint mismatch and insufficient resources, we could exit this function > sooner. > > Does that make sense and match your read of the code? > > [1] > https://github.com/apache/incubator-aurora/blob/091bd6decbb9d75962a55a4e5cff82ce58a2a064/src/main/java/org/apache/aurora/scheduler/async/Preemptor.java#L262 > [2] > https://github.com/apache/incubator-aurora/blob/091bd6decbb9d75962a55a4e5cff82ce58a2a064/src/main/java/org/apache/aurora/scheduler/async/Preemptor.java#L276 > [3] > https://github.com/apache/incubator-aurora/blob/091bd6decbb9d75962a55a4e5cff82ce58a2a064/src/main/java/org/apache/aurora/scheduler/async/Preemptor.java#L280 > > where the scheduler does unnecessary work by collecting preemptable tasks > on a host in the hopes that the SchedulingFilter is satisfied and no Vetos > are returned. > > -=Bill > > On Sun, Sep 14, 2014 at 12:40 PM, Stephan Erb <step...@dev.static-void.de> > wrote: > > > Hi Bill, > > > > you are right that the SchedulingFilter deals with constraints. However, > > the preemptor seems to be reacting poorly when a job cannot be scheduled > > due to such a constaint. Let me explain. > > > > The preemptor cannot distinguish whether he sees a veto because of a > > mismatched constraint or due to a lack of free resources on a slave. He > > assumes the latter and therefore begins preempting tasks until the whole > > slave has been drained. He returns happily, even though the task can > > still not be scheduled. > > > > At least that is the way I do read the code... > > > > Best, > > Stephan > > > > On So, 2014-09-14 at 08:49 -0700, Bill Farner wrote: > > > Thanks for raising the concern! > > > > > > > Unfortunately, it seems the preemptor cannot handle constraints > > > > > > Sounds like you are saying the Preemptor is only matching resources and > > not > > > constraints? > > > > > > I'm on a phone, so I may have missed something, but my read is that the > > > preemptor lets SchedulingFilter.filter deal with satisfying constraints. > > > > > > > > On Sunday, September 14, 2014, Stephan Erb <step...@dev.static-void.de> > > > wrote: > > > > > > > Hi together, > > > > > > > > I was scrolling through Preemptor.java and noticed that tasks might be > > > > preempted unnecessarily. > > > > > > > > If I am not mistaken, the code works as follows: > > > > > > > > > > > > for slave in slaves: > > > > while SchedulingFilter.filter is still yielding vetos: > > > > add preemptable task of slave to list toPreemptTasks > > > > > > > > if toPreemptTasks has some tasks: > > > > preempt > > > > return > > > > > > > > > > > > SchedulingFilter is checking for available resources and constraints. > > > > Unfortunately, it seems the preemptor cannot handle constraints: It may > > > > drain an entire slave trying to make room for a new task. However, if > > > > this task has a constraint which prevents it from being scheduled on > > > > this slave at all, then the other tasks are preempted unnecessarily. > > > > > > > > Is my analysis correct here? > > > > > > > > Best Regards, > > > > Stephan > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
