WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11649 closed defect (bug) (fixed)

Upgrades Fail Because of [11883]

Reported by: miqrogroove Owned by:
Milestone: 2.9.1 Priority: normal
Severity: blocker Version: 2.9
Component: Upgrade/Install Keywords: has-patch
Focuses: 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 (4)

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

Download all attachments as: .zip

Change History (15)

comment:1 hakre4 years ago

  • Keywords needs-patch reporter-feedback added

Related: [11883]

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

hakre4 years ago

comment:2 miqrogroove4 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.

miqrogroove4 years ago

Remove the SELECT statement from pre_schema_upgrade()

comment:3 miqrogroove4 years ago

  • Keywords has-patch added; needs-patch removed

comment:4 mrmist4 years ago

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..

comment:5 hakre4 years ago

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?

ryan4 years ago

comment:6 ryan4 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 ryan4 years ago

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

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

comment:8 ryan4 years ago

[12560] for trunk.

comment:9 miqrogroove4 years ago

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.

comment:10 hakre4 years ago

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.

comment:11 miqrogroove4 years ago

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.