search

humanmade / Cavalcade

A better wp-cron. Horizontally scalable, works perfectly with multisite.
https://engineering.hmn.md/projects/cavalcade/
Other
515 stars 46 forks source link

Retrieve job from cache in `Job::get()` #44

Closed igmoweb closed 6 years ago

joehoyle commented 7 years ago

The reason we don't have object caching here is because the Cavalcade Runner can change the database without memcached knowing about it. It doesn't (currently) run with the object-cache.php loaded (it's not a WordPress process). It's probably / maybe technically possible to do that though, but unsure if worth it.

As it stands right now, merging this would break a lot of things, @rmccue any idea if we want to try improve the cachability of the cavalcade db queries?

rmccue commented 7 years ago

cavalcade-jobs is a non-persistent cache (see discussion on #26), so this would only fetch from the cache if we're getting the same job repeatedly in the same process. It won't break anything, but I'm also not sure it'll have a huge effect on performance.

@igmoweb Without the persistent caching, is it worth doing this?

igmoweb commented 7 years ago

I missed that it was non-persistent one.

Definitely, it's not a big performance improvement but I still made the PR for the sake of consistency (jobs are cached so why not to get the jobs from cache on this function?) and because I was not sure how many times the function can be called in a single load. Now that I know that the cache key is non-persistent, the query is done at least once but could improve subsequent calls. Still, not a big deal and could not worth the risk unless we see cases where the function is being called many times.