Integrate with Features / CTools Exportables

I also require Features integration. I've only briefly glanced at the code, but I think it would make sense to have a dependency on CTools to make the cron rules exportable (and also to make your life easier with well-designed CRUD functions!)

If you agree that this makes sense, I am happy to submit a patch.

FYI I am coming from #1244162: Features compatibility- I was originally going to fork Elysia Cron, but Ultimate Cron seems to have a very good/similar feature set and much better code. I think if it just had Features compatibility, no fork would be required (much preferred for me!)

Hi Dane

Thanks for the nice words about Ultimate Cron. You are more than welcome to submit a patch regarding this, as I have not yet had the time to get myself that familiar with the exportables/features stuff. It's also something that I would very much like to see in Ultimate Cron. If you submit a patch, I will take a look at it asap.

Hmm, so I've hit a bit of a snag. In order to integrate with Features, you need a machine name key (i.e. "search_cron"). But the ultimate_cron_configuration table only has the numeric fid key. I can't tell why ultimate_cron_function and ultimate_cron_configuration are two separate tables... can we combine the two? Or else, can we at least add a 'function name' column to the ultimate_cron_configuration table?

I normalized the data, because of performance, when working with many rows in the log-table. However, it's more for historical reasons than anything else, and things have changed. Perhaps we could merge the two tables function and configuration. I would rather merge them though, than de-normalize the data.

From the top of my head, I propose a merged table just called ultimate_cron, containing fid, function and settings? I've been looking for an excuse to rename configuration to settings anyways.

Does Features really require the machine name to exist in the same table? I would suspect that Features looks at the exportable item as blackbox value with a machine name, and the modules integration maps it to a datamodel (e.g. multiple tables).

I think it is possible to have a CTools exportable span multiple tables, but it seems needlessly complex if we could just consolidate to a unified ultimate_cron table. Just FYI, I don't know if you currently implement any caching, but CTools automatically performs caching, so there shouldn't be any significant performance issues to joining the two tables.

EDIT: documentation on CTools exportables: http://drupalcode.org/project/ctools.git/blob/refs/heads/7.x-1.x:/help/e...

Yes, you're right. I'll merge the tables.

I've just made another release of Ultimate Cron, as there had been quite a few changes since last release, and I wanted to get them out before a potential refactoring of the tables.

About refactoring, I'm experiencing a bit of FUD. I want to keep the refactoring out of the dev branch for now.

Is it okay, if I just post a patch here, that you can use, or would you like to just do the table merge yourself?

Table merge patch...

Thanks- I will incorporate that patch into my own.

Alright, here's a preliminary patch. It should be fully functional and good for testing, but I think it still needs some work in places... here's a summary:

• Removes 'fid' field from ultimate_cron_logs, since we should use the machine name (function name) to uniquely reference functions from now on.
• Basically replaces all references to fid with function name, so that functions can be loaded from exported (code-based rather than db-based) Features. Note that this is necessary because exported functions have no primary ID (until UUID is implemented anyway).
• Some functions need optimization - specifically, ultimate_cron_update_7003() and _ultimate_cron_preload_cron_data(). I'm not sure exactly how to optimize _ultimate_cron_preload_cron_data(), since we basically have to JOIN ctools-loaded objects with conventionally selected objects

Hi Dane

Looks good. Some notes about the general design though:

• Is it necessary to remove the 'fid' completely? I deliberitely used an integer because of the log, since this scales much better than a string when having lots of logs. We currently run approx. 200 cron jobs each minute in average where I work, and this fills up the log table quite fast. Can't we just have the auto-increment in the ultimate_cron table?
• I'm not sure the update hook should update the background process and progress tables. The id's there are volatile, and the entries in those tables are for already running processes.
• Yeah, we probably need to optimize the preload_data. I need it to perform with at least 500 defined cron jobs.
• Ok, this one you are gonna hate me for (having read the Elysa thread where you came from) :-) ... I know CTools is a very common library, and almost "anybody who is anybody" has it installed. However, I think it's a shame to rely on this giant library, just for the get/set methods. I still want to implement the exportables feature so we can integrate with Features, but if there's ANY way to make it optional I would prefer this.

BTW: Shouldn't I create a new branch for this and provide you with git access?

Hi Dane

I've created a branch with your patch and granted you write access.

Branch: 7.x-1.x-features

For semi copy/paste pleasure:
git clone --branch 7.x-1.x-features @git.drupal.org:project/ultimate_cron.git

I've also committed some minor fixes to branch, but besides that, it seems to just work right out of the box with Features.

Hey gielfeldt-

fids
I'm not sure there's any good way to keep both Features compatibility and fids. The problem is that any function loaded from a Feature will not have an fid associated with it. The only workaround I can think of is to generate an fid for these functions using a hash or rand function, but that seems needlessly complex, and I'm not sure having a huge random number would be any more efficient than a ~10 character varchar.

I doubt that getting rid of fids for logs will have a huge performance impact anyway, because most interactions with the log are simply inserts- you only ever read from it if you're specifically looking through the logs, in which case performance probably isn't a huge concern anyway. Thoughts?

Updating background_process / progress
I didn't realize that those tables were so volatile- if you're sure they will eventually get recreated properly on their own without causing errors, I'm happy not to touch them - less code means less that can go wrong! :)

Ctools
I understand your concern. I'll see how feasible it is to make ctools optional. It might provide other useful benefits that might make it worth requiring. For instance, it has great caching that I expect will lower the database load a lot during cron calls (I haven't actually checked the number of queries / load to confirm this). It also provides the Export UI, which could take the place of about half of the code in ultimate_cron.admin.inc

Thanks for giving me access to the branch, I will start poking around there.

Hi Dane

I've now fixed some minor bugs and optimized the preload.

fids
I can see that fid still exists as an auto increment field for the ultimate_cron table. I guess we could use this as a reference to ultimate_cron_log table, but then we might break relations across imports. But I've tested the log table using the function name instead of fid, and it performs great. I've tested with +16M rows (equivalent of 400 cron jobs running each minute for 30 days), and there were no problems. So no fid in the log table is okay by me. I don't know if it still necessary in the ultimate_cron table, but it doesn't seem to hurt.

Updating background_process / progress
This is a tricky one. I think it'll break something in a running cron process whether we update the tables or not. I'll have to investigate this further.

Ctools
For now, I want to make ctools optional by just implementing the load/save and load-all and then switch between them based on ctools being available or not. This should be a fairly simple procedure to implement. If Export UI can be used in the future to replace much of the admin interface, then we can make a dependency.

But besides that, I'm very satisfied with this. Does it do what you need it to do, in the current state?

Hi again

I've removed the ctools thereby making the exportables support optional. I'll investigate the bit about updating the running background processes, but other than that, I think it's ready for a release. What do you think. Also, what's your view on versioning? Do you think it's necessary to bump the major? I personally don't think so, as it's only internal API that's been changed.

Updating background_process / progress
I think we should not update background process and progress tables during updates, as this might break cleanup of already running process. If we don't update, "only" ultimate crons logging will be affected.

Another thing perhaps to "worry" is the update of the ultimate_cron_log table. This can take a long time to execute depending on how many log entries you have, so it might timeout. Any ideas?

I mentioned that fid was still present in the ultimate_cron schema, but it is dropped manually in the hook_update(). This means that on updates the fid column will be removed, but on new installs, the fid column will be present. Should the fid column be available for ctools housekeeping, or what should we do with it?

EDIT: Disregard the italic above. The fid is only removed from the log table.

Hi Dane

I've removed the update of processes and progresses, and readded the indexes to the log table. Only problem I encountered was when running the update hook on a log table with 16M rows, it took about an hour :-o. But that's just the it is.

Comments? Otherwise, I'll merge it back into the dev branch.

Sorry I haven't had a chance to process this issue in full yet, I'll get back to you later... but just re: the 16M-row log table- yikes! How long are you keeping logs for? I believe the default is 30 days... even with 10 jobs running every minute that's still a half million rows, tops... could that be indicative of a bug?

No bug, I've tested with 400 jobs each minute. At work, we have 200 in avg. each minute. :-)

400 each minute = 17280000 log entries in 30 days (I did cut the test short at app. 16M rows, as I got impatient).

So no bug ... I'm just testing the scalability.

EDIT: The amount of rows I was testing with, was also one of my reasons for an integer based key. But it still seems to perform with a string. Only difference is the key size which gave me a table of 2.3GB instead of 1.6GB, but disk space is easy to scale, so thats fine, and if you don't have the disk space for it, you just turn down the cleanup age.

Cool- I've tested out the features branch and it seems good. Thanks for taking care of that!

I agree that a major version bump isn't necessary for this, but a minor release would be warranted as this is a pretty major new feature.

PS- sorry that I didn't see your previous post earlier about how many cron jobs you were running. That's pretty wild, but certainly explains how you could end up with 17M rows! :)

There! I've merged it back into dev. There are some other changes to dev branch since the last release, so I'll try out dev for a couple of days before making a release.

Thanks for all the help, it's a great feature added to Ultimate Cron.

And if you have the time, I would love some feedback on how Ultimate Cron works out for you.

Also, now you have git access, so you can always contribute if you like :-)