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

Using `post__not_in` in a query #1

Closed
philipjohn opened this issue Dec 3, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@philipjohn
Copy link
Member

commented Dec 3, 2015

In instant_articles_shortcode_handler_gallery() one of the queries using post__not_in which is not ideal as it can result in really expensive queries.

Is there another way we can do this?

One idea might be to create a hash of $atts and use that as a cache key to cache the result of the queries?

https://github.com/Automattic/facebook-instant-articles-wp/blob/master/shortcodes.php#L48

@bjornjohansen bjornjohansen self-assigned this Dec 8, 2015

@bjornjohansen

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2015

We are caching the entire article content until it is updated (or for up to a week), so I’m not sure whether we actually will gain anything by adding caching here.

From Greg Brown’s talk on Elasticsearch at WordCamp US, I got the impression that you on WPCOM are using ES_WP_Query for expensive queries like this. IIRC, post__not_in was even specifically used as an example in his slides.

Perhaps we should check if ES_WP_Query is available, and if so, use it instead?

@philipjohn

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2015

We are caching the entire article content until it is updated (or for up to a week),

I've missed that, sorry - can you help me out with where that is?

Perhaps we should check if ES_WP_Query is available, and if so, use it instead?

That'd be nice, though I'm unsure what'd happen if the plugin is available but no ES index is. Perhaps @gibrown could clarify?

@philipjohn

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2015

As an aside: had I realised you were at WCUS I would have sought you out! Next time...

@bjornjohansen

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2015

The cache checking is done here: https://github.com/Automattic/facebook-instant-articles-wp/blob/master/class-instant-articles-post.php#L163

Just a heads up: I have absolutely no experience with ES_WP_Query.

Also, I couldn’t make it to WCUS, but watched a few sessions on the live stream :-)

@philipjohn

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2015

Ah of course. Okay, that cache should be enough to avoid any explosions then, thanks!

Oh comfort of your own home and awesome talks. Good call. I won't tell you how great the after party you missed out on was then ;)

@philipjohn philipjohn closed this Dec 9, 2015

@izzuddinfz izzuddinfz referenced this issue Mar 14, 2017

Closed

Fatal Error #639

Blakomen added a commit that referenced this issue Jul 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.