#75 WP 5.1 Pre-flight filters #91
Conversation
The new filters will require additional hits to the database to determine job IDs. This warms the non-persitent cache with individual jobs to reduce unrequired hits to the databse.
#75 WP 5.1 Pre-flight filters (a few more things)
When it's used and updated in Cavalcade Runner, it doesn't have proper access to the object cache; in particular, we don't know (IIRC) the cache key generation algorithm. Because of this, the cache can't be authoritative. On my todo list to properly review this! |
|
Thanks for the explanation @rmccue. Seems kinda wild that the cache key generation isn't the same, that's a shame. I hadn't considered the runner environment. I think to move this forward we leave the cache as non-persistent and the db update in #95 will give us a performance boost in a different way. |
It's an internal implementation detail of the object cache, but we're not loading the cache at all in the runner, so we don't know how to use the cache (i.e. how does the runner know if you're using memcache or Redis?). We could potentially load it in, but we're then tying the runner heavily into how WP works; also, we're likely to hit memory errors due to the OC's in-memory cache. |
There was a problem hiding this comment.
We started testing this at work, @ffxluca discovered a back-compat issue.
| * } | ||
| * @return null|bool True if event successfully scheduled. False for failure. | ||
| */ | ||
| function pre_schedule_event( $pre, $event ) { |
There was a problem hiding this comment.
This will need to fire the schedule_event filter to match Core's existing behaviour allowing events to be changed prior to saving. (Work uses the hook to test cron events are registered properly so our tests started failing.)
There was a problem hiding this comment.
Added in 2c86202.
Per core, it allows the event to be changed after duplicates have been checked for.
There was a problem hiding this comment.
Aha, good catch @peterwilsoncc. I'll check for any other filters we're bypassing too
|
@peterwilsoncc @rmccue I also noticed in another place we're respecting any filters that run before ours but with this update we're not checking the value of See this branch for what I mean: https://github.com/humanmade/Cavalcade/compare/75-wp51-filters-fork...respect-the-filtery?expand=1 |
|
@roborourke Yeah, let's. I'm inclined to keep the hooks on |
Forked from #84
Fixes #75
Fixes #80