Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#10911 closed enhancement (fixed)

Optimize scan for lost attachments

Reported by: johnjosephbachir Owned by: westi
Priority: normal Milestone: 3.0
Component: Optimization Version: 2.8.4
Severity: major Keywords: has-patch tested
Cc:

Description

This patch moves all of the logic for scanning for lost attachments to the DB.

The code is more simple, and from my simple but repeated tests, the performance is significantly improved (done against a pretty sizeable posts table).

We could do it in one query instead of two if WordPress required MySQL 4.1, but subqeuries are not supported in 4.0 so we have to do it in two.

The patch was made against trunk, revision 11997 (last change to update.php, 11013)

Let me know if you have any questions or suggestions.

John

Attachments (3)

upload.php.patch (1.0 KB) - added by johnjosephbachir 4 years ago.
Somewhat more simple and possibly more optimal SQL.
10911.diff (776 bytes) - added by Denis-de-Bernardy 3 years ago.
untested
10911.2.diff (871 bytes) - added by Denis-de-Bernardy 3 years ago.

Download all attachments as: .zip

Change History (22)

Somewhat more simple and possibly more optimal SQL.

  • Keywords has-patch added
  • Milestone changed from 2.8.5 to 2.9

It's certainly an improvement.

comment:2   ryan4 years ago

2.9 will require 4.1, so subquery away.

comment:3   ryan3 years ago

  • Milestone changed from 2.9 to 3.0
  • Resolution set to fixed
  • Status changed from new to closed

(In [12655]) Optimize scan for lost attachments, props johnjosephbachir, fixes #10911

  • Resolution fixed deleted
  • Status changed from closed to reopened

we should use a subquery here, and the code is not custom post type safe.

  • Keywords needs-patch added; has-patch removed

untested

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

new patch strips parent-less attachments like the original query. see also #12448, on the same screen.

Can't test this it appears the scan is completely broken at the moment

  • Owner set to westi
  • Status changed from reopened to assigned

Initially this scan was designed to catch attachments where the parent post is deleted, i.e. post_parent > 0 but post doesn't exist, not orphan attachments with post_parent = 0.

Denis — open a new ticket if you want to advocate for including post_parent = 0.

Is the optimization portion of this done? Or did we want to combine into one query, using a subquery?

  • Severity changed from normal to blocker

The lack of custom post type awareness here causes any attachments to a custom post type be labeled as "lost".

The existing patch, assuming it works properly and within the bounds of defining what a "lost" attachment is, seems like the best solution.

(In [14701]) First pass at making the scan for lost attachments custom post type aware. See #10911.

  • Severity changed from blocker to major

Lowering severity after [14701].

The UX for lost attachments in general needs a little work, but that can wait. When you click "Scan", it refreshes the page, keeping you on the Untattached filter and not saying that you are viewing/searching lost attachments. If there are any found, it'll list those rows, otherwise, it'll list the unattached media. I digress.

Nonetheless, there should be limited unattached media as long as plugins and core cleans up after themselves.

However, the commit does introduce a quirk -- I personally find it a feature in more use cases than not, and think the ticket is ready to be closed as fixed at this point -- that a post type that has since been unregistered (say, plugin deactivated) will obviously not be in the query, and thus all attachments on those posts will be considered "lost."

The alternative is to simply do a post_type <> 'attachment' deal. But I'm not sure the current behavior shouldn't also be the intended behavior.

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

(In [14789]) Exclusion instead of inclusion for post types for lost attachment scan. Exclude attachments and any !public types, and don't consider attachments on unregistered post types to be lost. fixes #10911.

Note: See TracTickets for help on using tickets.