Ticket #11649 (closed defect (bug): fixed)

Opened 2 years ago

Last modified 2 years ago

Upgrades Fail Because of [11883]

Reported by: miqrogroove Owned by:
Priority: normal Milestone: 2.9.1
Component: Upgrade/Install Version: 2.9
Severity: blocker Keywords: has-patch
Cc:

Description

As described in great detail by several frantic webmasters at  http://wordpress.org/support/topic/342965

#2699 added a ridiculous SELECT then DELETE pattern to the upgrader, where only a DELETE was needed. As a result, the SELECT statement causes memory exhaustion and upgrade death.

Attachments

11649-distinct .patch Download (980 bytes) - added by hakre 2 years ago.
11649-proper.patch Download (964 bytes) - added by miqrogroove 2 years ago.
Remove the SELECT statement from pre_schema_upgrade()
11649-memory-is-the-cause-not-sql.patch Download (491 bytes) - added by hakre 2 years ago.
11649.diff Download (1020 bytes) - added by ryan 2 years ago.

Change History

  • Keywords needs-patch reporter-feedback added

Related: [11883]

an admin  is reporting to consider distinct in wp-admin/includes/upgrade.php.

hakre2 years ago

  • Keywords reporter-feedback removed

Yes it's a cute one-word workaround for desperate end users. In terms of database programming, it's turd polish. 2.9.1 should fix the problem not the symptom.

Remove the SELECT statement from pre_schema_upgrade()

  • Keywords has-patch added; needs-patch removed

If you really wanted a duplicate check before the delete, you could use something like

select min(option_id) from wp_de_options group by option_name having count(*) >1

Which may turn out more efficient than the joining method. Would also allow grouping by blog_id if that column is actuall used anywhere..

Replying to miqrogroove:

Yes it's a cute one-word workaround

Please tell that the admin who was reporting DISTINCT working in the forums. I only put that into a patch file since this ticket seemed important and the original reporter did not provide any more info / suggestions.

2.9.1 should fix the problem not the symptom.

Absolutely.

Q: How does 11649-proper.patch preserve the option with the highest ID?

Ups, I smeel a bit dataloss with your suggestion....


I thought over this now for some hours: The actual problem is not the select query but not having enough memory later on. Which means that results should be free'd. Might be the case anyway that the MySql database optimizes the query by it's own and we do not need to change it (in the meaning of that it's not the cause of the symptom).

Or am I wrong that those people in the forums all the time talk about memory problems? Or was it just guessed that the query is the cause of the problem and this has not verified yet?

ryan2 years ago

comment:6   ryan2 years ago

Patch fixes 11649-proper.patch. Verified that it removes duplicates, preserving the highest option_id. The SELECT in there now is a left over from when we used the IDs for other things.

comment:7   ryan2 years ago

  • Status changed from new to closed
  • Resolution set to fixed

(In [12561]) Better duplicate option delete query. Props miqrogroove. fixes #11649 for 2.9

comment:8   ryan2 years ago

[12560] for trunk.

How does 11649-proper.patch preserve the option with the highest ID?

That's what the WHERE condition is for.

Please tell that the admin

No worries. I was just distinguishing between a workaround and a full solution.

was it just guessed that the query is the cause of the problem

Most PHP configurations have a memory limit around 32 MB, so a pre_schema_upgrade() design that tries to calculate the Cartesian product of every ID in the options table and convert them all to strings is only going to work on relatively small tables.

Patch fixes 11649-proper.patch

Thanks, yeah I only got as far as testing with some quickie bogus column names.

So we healed the symptom now but nobody could find out until today why the users were reporting memory usage problems, right? For me that looks like we have a memory leak in WPDB related to get_cols. I think we should open a new ticket.

The actual problem is not the select query

As a qualified database programmer, I'm saying the select query is the actual problem. It's fixed now. Time to let this one go ;)

Note: See TracTickets for help on using tickets.