search

wp-cli / search-replace-command

Searches/replaces strings in the database.
MIT License
57 stars 45 forks source link

Address long-running queries and OOM contributor in PHP replacement #180

Closed brandonpayton closed 1 year ago

brandonpayton commented 1 year ago

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.

This is a resurrection of #176 which was closed so we could exercise these updates in production before sharing them with the community. 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.

Prior to this PR, there were tests for the chunked --precise search-replace but none for the chunked --regex path, so this PR adds tests for chunked --regex replacement based on the --precise tests.

danielbachhuber commented 1 year ago

Thanks, @brandonpayton !

I asked @schlessera for his thoughts about including this in the v2.8.0 release: https://wordpress.slack.com/archives/C02RP4T41/p1683671164356189

brandonpayton commented 1 year ago

Thanks, @brandonpayton !

I asked @schlessera for his thoughts about including this in the v2.8.0 release: https://wordpress.slack.com/archives/C02RP4T41/p1683671164356189

Thank you!

schlessera commented 1 year ago

GHA is acting up... :/

brandonpayton commented 1 year ago

Should I manually rebase and squash this one before merge? I guess it could be a way to try triggering GHA at least, but asking first so I don't get in your way.

brandonpayton commented 1 year ago

Squashed and pushed. Planning to leave this be for now unless you ask for other changes :) Didn't seem to trigger any actions.

schlessera commented 1 year ago

Thanks for the PR and the extended testing, @brandonpayton !