Add scheduled and recurring jobs #7
Conversation
src/System/Hworker.hs
Outdated
| , hwconfigFailedQueueSize :: Int | ||
| , hwconfigDebug :: Bool | ||
| , hwconfigBatchCompleted :: BatchSummary -> IO () | ||
| , hwconfigRecurringJob :: Hworker s t -> RecurringId -> Result -> IO () |
There was a problem hiding this comment.
So this part I'm a bit uncertain about. We have this function that gets called whenever a job with the particular ID is run. Three arguments are passed to it. The RecurringId so that you know what job it is, Hworker s t so that you can enqueue the next job, and the Result so that you can decide whether you want to enqueue a next job at all.
Initially I was actually trying to see if I could devise a way to do recurring jobs without any of this, by simply enqueuing the next job within the execution of a job itself. The problem I encountered is that all the enqueuing functions require Hworker s t and you don't actually have access to this from the job. One thought I had is that we could change job to the following?
job :: Hworker s t -> t -> IO ResultI believe that would allow us to just decide how we want to enqueue the job more directly and then we wouldn't need a RecurringId at all. But that would certainly break a bit of code. Granted, the change I've made instead might also break some code since HworkerConfig s is now HworkerConfig s t, though far less. Anyways, I'm trying to figure out which option makes the most sense or if there is a different option I haven't considered.
There was a problem hiding this comment.
This actually makes a lot of sense. I think you could actually remove the t parameter, since its contained within Hworker s t. The only downside is if there is repeated logic that each client would have to re-implement (and possibly get wrong) that you could do better by making recurring jobs a more first class thing, but it seems simple enough that that doesn't seem like an issue...
There was a problem hiding this comment.
My current implementation with hwconfigRecurringJob makes sense or the idea of just modifying the job function?
There was a problem hiding this comment.
modifying job to have type Hworker s t -> IO Result (or keep the t, but it's just hworkerState hw where hw is the Hworker s t argument, right?).
There was a problem hiding this comment.
Yeah, so my thinking is that for now I just want to provide the bear minimum for one to be able to do recurring jobs. Maybe once I actually start trying to implement it in one of my applications it'll become apparent that recurring jobs should be more of a first-class thing and at least I'll have a better idea at that point of what that should look like.
| runRedis (hworkerConnection hw) $ do | ||
| R.zcount (scheduleQueue hw) 0 (utcToDouble now) >>= | ||
| \case | ||
| Right n | n > 0 -> | ||
| withNil' hw $ | ||
| R.eval | ||
| "local jobs = redis.call('zrangebyscore', KEYS[1], '0', ARGV[1])\n\ | ||
| \redis.call('lpush', KEYS[2], unpack(jobs))\n\ | ||
| \redis.call('zremrangebyscore', KEYS[1], '0', ARGV[1])\n\ | ||
| \return nil" | ||
| [scheduleQueue hw, jobQueue hw] | ||
| [B8.pack (show (utcToDouble now))] | ||
|
|
||
| Right _ -> | ||
| return () | ||
|
|
||
| Left err -> | ||
| liftIO $ hwlog hw err | ||
|
|
There was a problem hiding this comment.
So I wasn't sure if this was at all necessary, but since this monitor is polling pretty frequently and most of the time there won't be anything in the scheduled queue. I figured I'd have it at least do an initial check to see if anything is there before bothering to run the Lua script, since I know that's blocking. But maybe this is pointless?
No description provided.