caching for project_release_table()

Thanks for taking an initial stab at this! To answer your questions and help clarify some things...

cache_clear_all(NULL, 'cache_project_release');

Right, that shouldn't be in every invocation of package-release-nodes.php.

"Is the packaging script invoked directly by cron?"

Yes, by UNIX cron, completely outside of Drupal. It runs as a different user (with different permissions) than the httpd user. package-release-nodes.php is a CLI Drupal bootstrap script, not something to ever run from the web.

"Where does the list of project nids come from then?"

There's a magic command-line arg to only work on a single project, which is used for debugging and the occasional support request (e.g. changing the version info for a snapshot release and then rebuilding the tarball, etc). However, if you leave that off (which we do from cron), then the script itself does a big nasty query to find all the release nodes that need to be packaged for all projects -- see package_releases().

"I don't see a hook_cron in project module."

That's right. It's the UNIX crontab you're looking for, and you can't see it. ;) The script is invoked in "tag" mode every minutes (which only builds new official releases), and in "branch" mode every 12 hours (regenerates tarballs for any projects that have updated files).

So, I'd say:

A) package-release-nodes.php could keep track of each project for which it built a new tarball. When it's done, it could clear everything from {cache_project_release} for any cids that match the effected projects.

B) I wonder if {cache_project_release} isn't specific enough, and we should call it {cache_project_release_table} just to be sure?

C) You should only actually clear all when invoked for branches, not on every run.

Other things I noticed when skimming the patch:

D) Why doesn't the mysql case in the .install also check if the table already exists?

E) Shouldn't project_release_update_5000() test if the table already exists, too?

For some reason the existing install code checks table_exists for postgreSQL not MySQL, so I just mimicked this.

Not true. The existing install code just checks in a different way. On MySQL, you can do CREATE TABLE IF NOT EXISTS. PgSQL doesn't have that, so you need the extra call to db_table_exists(). Please update the install (and upgrade) code to handle this. Thanks!

As for naming the table - I'd think this cache table could potentially have other uses for this module, so it didn't seem necessary to give it the longest possible name.

Ahh, ok, that explains why you have "table" in the cache index string, then. ;) Makes sense to me.

Final point on cache clearing during the packaging script. You're almost there, but I think I'm not explaining what I have in mind very well. ;) Lemme try again...

- Packaging script always knows when it creates a new tarball for a given project.

- A new tarball is really the *only* reason to ever clear this cache info. If a project hasn't had any commits, the cached release table entry could be valid for weeks on end, not just 12 hours.

- I'm assuming that if a given project had multiple new tarballs generated (which is often the case -- if someone committed some changes, it's often to fix a bug and the patches get backported, etc) that it would be better to only try to clear that project's cache once, instead of re-clearing it for every tarball.

- Right now, package-release-nodes.php doesn't SELECT the project's nid in the query. We only know that if it was supplied on the command-line for a single project. However, it'd be trivial to change the SQL so we had the project nid, even when we're querying for all the release nodes that might need to be (re)built.

So, the kind of changes I have in mind would be a little bit more far-reaching in package-release-nodes.php, but I think the extra effort is worth it so that we don't clear the cache unnecessarily. I'll try to find you on IRC to coordinate further -- might be faster for me to just take your latest patch and crank out a new one with what I have in mind...

Thanks again for working on this!
-Derek

Almost. ;) More like this.

This was minorly broken in a few ways, mostly because I led pwolanin astray at various points... ;)

A) The cache id was wrong for clearing the cache for a specific project. We both left the 'table:' part off the front. ;)

B) We should really clear the cache for a project whenever a release node is added or updated, too, since that could effect various things.

C) We should also clear the cache if the project owner tweaks the settings on the "Releases" subtab of the edit page (e.g. to change the default major version for a given version of core).

New patch fixes all this. I tested the upgrade path, the fresh install, and a bunch of cases and everything looks great. I haven't yet tested the changes to the packaging script to clear the cache, so that still needs some testing...

Did some more testing, and committed to HEAD. Just installed on d.o. Ran into an evil problem where, because d.o is now running memcached, the schema for cache_* tables has to be different. :( This caused an explosion of watchdog errors for a little while, until I undid the patch, manually altered the schema in the DB, and reapplied. Now, everything is running smoothly. I verified that project nodes look ok and that the {cache_project_release} table has some records now (1630 and counting). ;) Thanks!