Address long-running queries and OOM contributor in PHP replacement#176
Address long-running queries and OOM contributor in PHP replacement#176brandonpayton wants to merge 1 commit intowp-cli:mainfrom
Conversation
This change makes PHP replacement queries and memory usage more efficient. It improves the primary key SELECT queries by eliminating the use of OFFSET because OFFSET requires that the database consider all rows up to OFFSET before taking rows up to the LIMIT. The new query relies on primary key conditions to more efficiently eliminate previous rows from consideration. This way, the database can use an index to identify rows with keys greater than those of the previous chunk. It improves memory usage by doing updates along the way rather than storing all a column's updates in memory until the end. At Automattic, when we limit search-replace to 4GB of memory, we sometimes exceed that limit for large sites. It's possible there are other things that contribute to high memory usage within the search-replace command, but as a first step, we can reduce memory requirements by no longer keeping all updated column values in memory simultaneously.
|
@brandonpayton Has this been running in production for some time? Also, are there any particular test cases you think we should add to cover the change? |
|
@danielbachhuber, this is not running in production yet but should be next week. I'll withdraw this PR for now and plan to resubmit later after it has been in production for a while. I figured the current test cases would cover this code as the resulting replacements should not change, but it would be a good idea for me to review the tests and confirm that. I can do that before resubmitting the PR. Thanks! |
@brandonpayton Sounds good, thanks!
I think the functional existing tests are pretty good but may or may not be comprehensive enough to cover this. It could be fine or there could be some minor idiosyncrasy that causes problems. |
|
|
||
| foreach ( $updates as $update ) { | ||
| $wpdb->update( ...$update ); | ||
| // Because we are ordering by primary keys from least to greatest, |
There was a problem hiding this comment.
Love it, 2 wins. Runs faster than limit/offset and solves the memory problems. Thanks for working on it. 🙇🏻
|
The regex path of this change has been used in production for the last 3-4 months, and we have encountered no issues so far. So I am reopening this PR for consideration. We already had tests for the chunked precise search-replace but none for the chunked regex path, so I added some based on the "precise" tests. |
|
Actually, I do not have permission to reopen. :) @danielbachhuber, are you up for reopening this PR? |
|
@brandonpayton It seems like the branch disappeared for me too: 
Want to create a new PR? |
It was the originally stated plan anyway, but I figured "why not just reopen?" I guess this is why :) Thanks for looking, @danielbachhuber! |
This change makes PHP replacement queries and memory usage more efficient in order to address issues we've encountered with long-running queries and OOM conditions with certain sites.
It improves the primary key SELECT queries by eliminating the use of OFFSET because OFFSET requires that the database consider all rows up to OFFSET before taking rows up to the LIMIT. This can make queries become slower as OFFSET increases. The new SELECT query relies on primary key conditions to more efficiently eliminate previous rows from consideration. This way, the database can use an index to identify rows with keys greater than those of the previous chunk and then take rows from that set up to the LIMIT.
It improves memory usage by doing updates along the way rather than storing all a column's updated values in memory until the end. At Automattic, when we limit search-replace to 4GB of memory, we sometimes exceed that limit for large sites. It's possible there are other things that contribute to high memory usage within the search-replace command, but as a first step, we can reduce memory requirements by no longer keeping all updated column values in memory simultaneously.