JPry
commented
6 years ago I think this makes sense to adjust, and I anticipate that the typical site using this will see a benefit.
we can just release a plugin using the available filters for larger sites where we might want to migrate things faster.
Conversely, we can also release a plugin using the available filters for sites that can't handle this to migrate slower.
Given the real world experience, I think this is still conservative enough to warrant merging, while also providing a benefit.
To run the scheduled migration event more frequently and migrate more actions in each batch.
I'm not sure why the schedule was originally set to be 5 minutes apart, but it seems like way too much time between processing. I'm guessing it was done to avoid duplicate queues doing a migration at the same time, and/or to make sure it ran after the built-in timeout mechanism.
Now that https://github.com/Prospress/action-scheduler/issues/108 is implemented, I'm really not sure we need to give 5 minutes anymore.
Regarding batch size, on a (real) large site's database, it was taking less than 30 seconds to migrate a set of 1,000 actions (with 10,000 actions already in the new table and a post table with near 1,000,000 rows and post meta table near 50,000,000 rows). So it seems like 1,000 inserts in one request should be manageable. Even on lower powered servers. I think the previous thresholds were set simply to be conservative (which is reasonable given we have the Hybrid data store).
Defaulting to a Conservative Approach
I am still in favour of taking a very conservative approach to the amount of data, and frequency of which we upgrade to avoid any potential issues across sites. If there's even remotely potential issues with these changes, then it should not be merged. Especially as we can just release a plugin using the available filters for larger sites where we might want to migrate things faster.