search

chrisboulton / php-resque

PHP port of resque (Workers and Queueing)
MIT License
3.43k stars 759 forks source link

Question about the default value of $isTracking in class Resque_Job_Status #207

Open XIN-Dongzhe opened 10 years ago

XIN-Dongzhe commented 10 years ago

I'm a little confused by the default value of the property $isTracking in Resque_Job_Status...so is there any difference if it's set to 'false' instead of 'null' when it's initialized for the first time?

danhunsaker commented 10 years ago

It's set to NULL instead of FALSE so we can tell the difference between "haven't checked Redis yet" and "didn't find the key in Redis, so it isn't being tracked". If we haven't checked, we want to. If we have, there's no sense checking again.

XIN-Dongzhe commented 10 years ago

So that's clear. THX ^^! And yet another problem...I notice that the parameter $trackStatus passed to function 'enqueue' is only used once in deciding whether to create a new 'Resque_Job_Status', and when the function 'isTracking' is called it is not used anyway. So maybe we can make it serialized as well with the task so that when 'isTracking' is called it is checked first and then the Redis if necessary. I've noticed that when I set $trackStatus to FALSE there are still two additional 'Redis::exists' operations and I think in the case that we do not monitor the task there is no need to check Redis(though it should not be a bottleneck anyway ==). Hope that my question is not a picky one...or is there any special meaning in using $trackStatus only once?

danhunsaker commented 10 years ago

My only real concern with that is the increased complexity in removing the job once it's been queued, but we have that issue anyway now that we've added the queue_time value. To remove a specific list item in Redis (without using the normal pop mechanism, that is), you need to provide the exact value you're looking for. That means in order to cancel a job before it runs, we need to have the queue name, the job ID, the job payload, and in the latest master, the queue time with microseconds. This makes canceling jobs rather difficult already, since you have to know everything about the job you're trying to cancel in order to cancel it. Adding another value, which would only reduce the Redis lookups by two (not sure why there are two, though...), doesn't seem much more efficient. Granted, there is room for improvement, but I'm not sure this is the right way to do it.