cozy / cozy-controller

Cozy Module and Apps deployer
cozy.io
GNU Affero General Public License v3.0
5 stars 13 forks source link

Add queue management layer #35

Open m4dz opened 9 years ago

m4dz commented 9 years ago

In this PR we pint that apps could be able to register some delayed tasks to a global task layer that can execute the task in another timeframe (which can be just another thread called immediately or that can be delayed to another date).

The app should:

I suggest to rely on kue for the queue manager.

frankrousseau commented 9 years ago

Thank you for that interesting input. The image data processing could be redesigned with that job queue. But I think it's the role of the data-system to transmit information to other apps. Why did you think about the controller?

jsilvestre commented 9 years ago

kue is based on redis btw, I'd rather go for a custom implementation to keep things simple, a couple of setTimeout and if we really need a queue, async.queue will do the job. Do we actually need something more powerful?

About where it belongs: I think the API should be in the Data System, mostly for practical reasons (app authentication is already managed there, apps only talks to the data system), then what should the job it's up to us to decide in function of the job itself: if it's just starting an application (say to retrieve new data, like Konnectors, Emails, PFM), the controller seems to be suitable for that. For image management, the Data System itself looks better, but it could even be a separated service that we add to the stack (although performance-wise it's not optimal).

Anyway, here are the current parts of Cozy that could find this useful:

aenario commented 9 years ago

my two cents :

m4dz commented 9 years ago

I primary though at controller, because, as you said, it manages apps. So it can easily start/stop/wake up an app, or call it to execute a task (i.e. wake up Konnectors and tell it to perform auto-imports). But I agree that DS already handles auth and several internal runtimes. You better know the stack than me :smile:.

I do not agree on "simpler is better" in this case: if we only use a couple of setTimeout, we will risk to loose the tasks queued when the task app director restart or crashes (which may be problematic for delayed tasks such as alarms or imports). Rely on a broker (redis or whatever) will ensure that if our app stops, it can retrieve its tasks lists to continue its jobs, transparently for the user. I'm not so attached to kue, but I think a solution which only use setTimeouts may be too simple.

aenario commented 9 years ago

Yes, we probably should not use setTimeout, however the DS can store this informations as a docType, something like

ApplicationWakeUpCall
   timestamp: Number 
   app: String # slug
   createdBy: String # slug

The DS simply create / update this docType (enforcing that apps can only change the wakeups they created). When the controller start, it can fetch this docType list, awake the missed ones and setTimeout for the other (similar to what home do for alarms).

m4dz commented 9 years ago

Yeah, using the DS as a broker is a very good idea!

Do we expect some performance issues when using multiples timeouts running inside the controller for pending tasks ?

aenario commented 9 years ago

Well timeout is cheap cpu-wise but retains its inner-function context.

so

listAllApps (err, apps) ->
     setTimeout ->
         doStuff(apps["files"])
    , 300

Retains the whole apps object (which is probably already somewhere in the controller memory). However,

listAllApps (err, apps) ->
     setTimeout doStuff.bind(null, "files"), 300

only retains the "files" string.

jsilvestre commented 9 years ago

Our inter-app mechanism would work great with that: an application create a ApplicationWakeUpCall document, the application in charge of waking apps up listen to those and do the job. That way, it's decoupled from a particular app (we can move it easily elsewhere if we want), there is no new developer facing API to create (to maintain, and to document), besides the one from the doctype itself. Also like Romain said, it's fault tolerant because it is in database. But it's still dead simple.

aenario commented 9 years ago

Mmmh, problem with using the normal API is that a bad application need to ask permission for this weird docType the user doesn't understand and then can decide to do a destroyAll on them.

jsilvestre commented 9 years ago

Which is a more global issue, that we've been ignoring since the beginning (so why would it change <= my reasonning).

aenario commented 9 years ago

Or we can stop digging,

  1. give the DS an API for this topic
  2. give the DS some API for getting timezone / owner email (prevent asking for user & cozyinstance)
  3. at some point in the future, use (Web?)Intent(-thingy) to handle clearance (prevent asking for contacts)

Or do you mean the issue of non separation of read / write permissions ?

jsilvestre commented 9 years ago

The whole problem with permissions.

edit: the one API/issue is fine too, but I'd rather stop pushing new stuff, and fix the existing one if it's possible/needed