WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 10 months ago

#24688 new defect (bug)

Memory exhaustion caused by very many unattached media

Reported by: andy Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.5
Component: Media Keywords: has-patch needs-testing
Focuses: Cc:

Description

A blog with over 30,000 unattached images was exhausting available memory on unattached attachment pages. The exhaustion occurred in previous_image_link() -> adjacent_image_link() -> get_children() due to the large number of rows selected with post_parent=0.

This is not the only place where get_posts() might be called with post_parent=0. A similar query appears in twentythirteen_the_attached_image() but the effect is wisely mitigated by fields=ids. Earlier, twentyten/loop-attachment.php called get_children() with post_parent=0 on unattached image pages which is horribly inefficient when there are many unattached images.

Attached patch depends on #24687. The change to adjacent_image_link() creates a MySQL query which selects only the desired row. This fixes the memory exhaustion error on the blog with 30,000 unattached images. The once-only filters would be unnecessary if WP_Query accepted arbitrary extra WHERE strings.

Attachments (2)

24688.diff (2.4 KB) - added by andy 4 years ago.
media.patch (3.0 KB) - added by dllh 10 months ago.
Updated patch

Download all attachments as: .zip

Change History (13)

@andy
4 years ago

#1 @SergeyBiryukov
4 years ago

  • Component changed from General to Media

Related: #23044

#2 @nacin
4 years ago

  • Keywords has-patch 3.7-early added
  • Milestone changed from Awaiting Review to Future Release

#3 @SergeyBiryukov
4 years ago

  • Version changed from trunk to 2.5

Related: [7222]

#4 @wonderboymusic
4 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#5 @ocean90
4 years ago

  • Milestone changed from 3.7 to Future Release

Punting due to lack of testing.

#6 in reply to: ↑ description @ericlewis
3 years ago

Replying to andy:

The once-only filters would be unnecessary if WP_Query accepted arbitrary extra WHERE strings.

That's interesting. Are you subtly proposing this idea?

#7 follow-up: @kitchin
3 years ago

About the patch
return "$where AND (...
should be
return "( $where ) AND (...
There are operators that bind less tightly, like OR.

Is that what you meant by testing? :)

#8 in reply to: ↑ 7 @andy
3 years ago

I think since #26042 is closed this ticket now just needs a new patch and testing.

Last edited 3 years ago by andy (previous) (diff)

#9 @chriscct7
2 years ago

  • Keywords needs-patch added; has-patch 3.7-early removed

@dllh
10 months ago

Updated patch

#10 @dllh
10 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

I've updated the patch for trunk and given it a test drive. It seems to work well for me. Any chance of nudging this along?

#11 @kitchin
10 months ago

My objection above still stands, to $where without parentheses in return "$where AND ...

Note: See TracTickets for help on using tickets.