[Ovirt-devel] Re: Thoughts about taskomatic redesign

[David Lutterkort]

On Fri, 2008-06-27 at 15:16 -0400, Scott Seago wrote:
> David Lutterkort wrote:
> >
> > Probably another thing that's best left as 'future work'. From your
> > description of existing tasks, it seems that keeping them close to the
> > WUI interaction is probably the simplest for now.
> >
> >   
> That's fine. Without this, though, it might be harder to define 
> dependencies, since the 'clear this host' tasks will have a much more 
> complex dependency graph. i.e. any prior or subsequent queued tasks on 
> any of the VMs on this host will block on this. If we have subtasks, 
> then if a host has 15 VMs on it, later vm tasks for those VMs will only 
> block until each individual VM has migrated rather than waiting for the 
> whole lot of them to complete.

Yeah, I think queueing a migrate task for each VM and having the 'clear
the host' task depend on each of them is the right thing to do here; so
that the 'clear_host' API call would do something like:

        mark host unavailable
        queue a migrate task for each VM on that host
        queue a 'clear_host' task and have it depend on each migrate
        task

The action for the clear_host task would actually do very little, maybe
flag the host as 'cleared' in the DB if that's needed. Assuming a
fictitious 'reboot_host' API call, its logic would be something like

  if host is clear:
    queue reboot_host task
  else:
    do clear_host from above
    queue reboot_host task with a dependency on clear_host

Tracking of the parent task/subtask relation is really only important
for UI purposes, it shouldn't complicate things too much to track that
in each of the tasks.

This example also brings up another point: the actions for each task
need to be tolerant to the fact that their work has already been done;
for example, if you call clear_host twice for the same host, you'd end
up with two migrate tasks for each VM on that host. The action for the
migrate task needs to be deal with the fact that the VM might already
have been migrated away, and succeed without doing anything.

It's tempting to catch such double-queuing when tasks are added to the
queue, but I think making tasks state-oriented as much as possible
rather than action-oriented will be much more robust. What I mean with
that is that the meaning of the migrate task is 'make sure this VM runs
somewhere else than on host X' rather than 'call into libvirt to migrate
the VM away'.

> >>  When the WUI insterts a task for "create 
> >> this VM" or "migrate this VM" the dependency information will not be 
> >> known by the WUI. So something on the taskomatic side will have to, as 
> >> part of the "notice new tasks in the queue and process them"
> >>     
> >
> > That's an important point and reason to separate the task queueing logic
> > a little more from the WUI, so that instead of just creating and saving
> > a new task object to the DB, you actually have some sort of API for
> > adding new tasks, and the implementation of the API knows what possible
> > dependencies and prerequisites to look for for each task.
> >
> >   
> True. There are two steps needed here. "insert this task" and "figure 
> out what it depends on". There are various ways to model this:
> 1) queueing API figures out current deps based on what's in the queue 
> before (or perhaps just after) insertion
> 2) queuing API just inserts the task and whatever API looks for new 
> tasks does the dependency checking.

I think (1) makes more sense, since depencies are heavily dependent on
what the actual task, i.e. 'start_vm' has totally different dependencies
from 'clear_host'.

> It actually probably makes more sense for 1) to happen. Although this 
> API could just be built into the Task ActiveRecord model -- then the WUI 
> would just call Task.insert rather than Task.new.

I think this logic has to live one step above AR - it's more than just
inserting a bunch of rows into the DB. For example, for the 'start_vm'
API call, you want to perform a few checks before even queueing the task
(like: is there quota available for this VM, is the storage setup resp.
is a task for that queued etc.) and fail the API call immediately if any
of those preconditions is violated. Unfortunately, you'll have to do
those checks _again_ when you're executing the task.

So all this belongs more into a controller than the AR model.

> > That API would probably also be the right one to expose publically.
> >
> >   
> Right.  Perhaps this public API would be a layer between the current 
> ruby model and controller classes? (then exposed via REST or whatever)
> -- or would the APIs be built into the model classes themselves (again 
> exposed in the same way)?

I don't feel like they belong into the models; controllers seem the
right place. 

David