Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added suppress_filters param to get_posts / get_children function calls. #8625

Merged
merged 1 commit into from
Feb 27, 2018
Merged

Added suppress_filters param to get_posts / get_children function calls. #8625

merged 1 commit into from
Feb 27, 2018

Conversation

spacedmonkey
Copy link
Contributor

Fixes #8624

Changes proposed in this Pull Request:

  • Pass suppress_filters to all public facing calls to get_posts / get_children

Testing instructions:

  • This is hard to test. There should be no noticeable difference it it works.

Proposed changelog entry for your changes:

Added suppress_filters param to get_posts / get_children function calls.

@spacedmonkey spacedmonkey requested a review from a team as a code owner January 26, 2018 12:58
@zinigor
Copy link
Member

zinigor commented Jan 26, 2018

Thanks for the contribution!
You are saying that this is hard to test, but in the issue you have provided examples of plugins that may rely on filters not being suppressed. You probably had one or two cases where this change would fix a problem. Can you at least describe that case? Otherwise if it makes almost no difference, as you say, we don't see why it needs to get in.

@zinigor zinigor added General [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jan 26, 2018
@spacedmonkey
Copy link
Contributor Author

Thanks for getting back. Advanced post cache is the perfect example of a use cache then. Out of the box, WP_Query runs a couple of queries and these are uncached. Advanced post cache, used on all WordPress.com sites (even GO VIP), hooks into WP_Query can caches the result of these queries to stop the running on every page load. With this suppress_filtersadvanced post cache doesn't not run. This means queries run unnecessarily and result in high level of traffic to database servers at peak times.

Take for example this query that is running on production.
screenshot from 2018-01-26 14-32-03
This query should be cached, specially because it is a LIKE search.

It is hard to test because you have to be running another plugin to take advantage of the performance win here.

@zinigor zinigor added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Jan 26, 2018
Copy link
Member

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds reasonable 👍

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 20, 2018
@oskosk oskosk added this to the 5.9 milestone Feb 23, 2018
@zinigor zinigor merged commit d7d094c into Automattic:master Feb 27, 2018
oskosk added a commit that referenced this pull request Feb 27, 2018
oskosk added a commit that referenced this pull request Feb 27, 2018
* update changelog.txt

* Update readme.txt with scaffolding for 5.9 changelog and release draft shortlink

* Add changelog entry for #8243

* Add changelog entry for #8296

* Add changelog entry for #8367

* Add changelog entry for #8686

* Add changelog entry for #8707

* Add changelog entry for #8709 and #8714

* Add changelog entry for #8729

* Add changelog entry for #8777

* Add changelog entry for #8780

* Add changelog entry for #8786

* Add changelog entry for #8787

* Add changelog entry for #8801 #8805 #8832 #8865 and #8804

* Add changelog entry for #8817

* Add changelog entry for #8822

* Add changelog entry for #8823

* Add changelog entry for #8829

* Add changelog entry for #8834

* move some items to major enhancements

* Add changelog entry for #8836

* Add changelog entry for #8839

* Add changelog entry for #8861

* Add changelog entry for #8862

* Add changelog entry for #8863

* Add changelog entry for #8866

* Add changelog entry for #8870

* Add changelog entry for #8874

* Add changelog entry for #8875

* Add changelog entry for #8881

* Add changelog entry for #8890

* Add changelog entry for #8911

* Add changelog entry for #8927

* Add changelog entry for #8931

* Add changelog entry for #8933

* Add changelog entry for #8930

* fix wording

* typo

* minor fixes

* replace partner scripts for Jetpack Start in changelog entry

* Update to-test.md

* Update to-test.md

* minor style fixes to to-test.md

* minor style fixes to to-test.md

* minor fixes on to-test.md

* Add changelog entry for #8868

* Add changelog entry for #8844

* Add changelog entry for #8664

* Add changelog entry for #8935

* Add changelog entry for #8425

* Add changelog entry for #8625
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass suppress_filters param to all get_posts calls
7 participants