WordPoints / wordpoints

Points plugin for WordPress
GNU General Public License v2.0
20 stars 15 forks source link

Multisite (un)install may not scale #68

Closed JDGrimes closed 10 years ago

JDGrimes commented 10 years ago

Our install and uninstall routines for multisite may not scale on large networks. This may be a wontfix, but let's at least consider possible solutions.

For the install routine, our only concern is network-activation. We could do the network-wide installation actions, but not run the per-site installation until each site was accessed. This would, however, require that we run a check for installation each time, which I would like to avoid, if possible.

On the other hand, we already do this for updates, and updates will need to receive this treatment as well. So maybe we should just tweak the update code to run install too when in network mode.

For uninstallation, this is not a possibility. We have only one chance to run our uninstall script, then we're deleted. One thing we could do is detect large networks, and just not run the per-site uninstall for them. Note however, that the uninstall script doesn't currently have any knowledge about whether the plugin was in network mode or not, so it would run for any multisite installation. Considering that, it would be a shame to skip the uninstall, when only one site actually had WordPoints activated anyway. Likewise, it is really silly for us to run the uninstall routine for every site, when perhaps only one needs it. To solve this we could introduce a new option in the database that stores a list of sites where the plugin has been activated, or whether the plugin was in network-mode instead. (Or, perhaps site meta would suit this better?)

JDGrimes commented 10 years ago

Related #67

JDGrimes commented 10 years ago

I thought of a problem with the proposed activation solution. If we only do the per-site install when each site is accessed, this could cause strange things to happen when using switch_to_blog(). So we would likely have to run our check each time a blog was switched to as well, but that could defeat the purpose entirely if another plugin is looping through all blogs on a large network.

In addition, I don't think it is very likely that network mode would be used on a large network, and especially not an already-large network.. Perhaps maybelater on the activation part is the best thing for now, and lets focus on the uninstall stuff.

JDGrimes commented 10 years ago

As far as our update process goes, we should probably at least change it so that we only loop through each site once when in network mode. Of course, if we do end up changing activation, this may be a moot point.

JDGrimes commented 10 years ago

For activation, we could also rely on something like WP Proper Network Activation. But it wouldn't really be backward compatible to switch to that, and it doesn't solve the uninstall or update issues.

JDGrimes commented 10 years ago

Related: WP#14170

JDGrimes commented 10 years ago

First step:

JDGrimes commented 10 years ago

There is one issue with that. We won't know about which sites we were installed on previous to the update. We could just ignore that, or try to handle it on update.

JDGrimes commented 10 years ago

I'm thinking about the difference with storing an array of sites as a network option versus storing an option for each blog. If we do it the latter way, I guess that means we'd have to switch to each blog to retrieve the option, which would kind of defeat the purpose. The problem is that the former way of doing will end us up with a massive option value on large networks.

So now I'm considering ways to get out of this altogether. While not running the install when network active might seem really lame, it is probably a good idea if we're on a large network. The install in that case is probably best left up to the network administrators, and they'll probably be doing it manually in most cases anyway. The same can go for uninstall. The question is, what about updates?

TL;DR: There is no good way of handling un/install on large networks; it is best left up to the administrator.

JDGrimes commented 10 years ago

We should probably notify the user when the install is skipped. I was thinking at first about just logging it in the PHP error_log file, but we could make it a part of the UI. I was going to say that we could go as far as warning them before the activate the plugin, but that probably isn't possible. It might be worth looking into though. Also, we probably can't tell the user that we didn't uninstall, because at that point, we're already gone (though we could log a notice like I said above).

I'm thinking that, for the most part, updates are going to have all of the same pitfalls as un/installing. So I think that we should generally skip them as well when network active on a large network. We could give the admin a notice or log a notice when we skip the upgrade routine.

Now for one other issue: being installed on a lot of sites on a large network. Install is okay there, because that is run per-site (for site-specific stuff). But we still have issues for update and uninstall. Maybe this is something that we should revisit later, once we can determine how many sites we are installed on.

JDGrimes commented 10 years ago

I've decided to refactor the install and uninstall code into a class, which provide a bootstrap for the plugin, components, and modules.

I've also realized that the ranks component doesn't uninstall itself fully. I'll create a ticket for that.

JDGrimes commented 10 years ago

The only thing left to be done here is to run an update for 1.8.0 that will make sure that the active sites are added to the list on multisite installs where the plugin isn't network-activated so that uninstall will run correctly. That might require us to loop over all sites on the network. Obviously, we won't do that if it is a large network.

JDGrimes commented 10 years ago

One other thing not handled here is what to do on a large multisite network where we aren't network active but have been activated on a lot of sites. In that case we shouldn't attempt to do the per-site uninstall. Updates and install aren't an issue, since they will be run only when that site is accessed.

Also, on multisite and when network-active, we don't automatically install on a new site when it is added. That currently has to be implemented manually by each component. I'd like to consider ways of automating that as well.

JDGrimes commented 10 years ago

I've thought of something else. When we're activated per-site on multisite and some of the sites where we have been installed get deleted there will be database errors when we uninstall. We'll need to check if each site ID is valid, or remove them as sites are deleted. The former is more fail-proof, and maybe simpler to implement. There are two ways that we could do it. We could check each site as we loop through them, or we could check them all at once before we start. The later might be better, as it would reduce the number of database queries considerably, though of course that single query would be more intensive.

JDGrimes commented 10 years ago

I think we'll call this fixed. I've forked off my one unimplemented suggestion above into a new ticket.

JDGrimes commented 9 years ago

I've added the related docs.