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

SOLR-9: Add ASIN to release results #31

Merged
merged 1 commit into from Oct 12, 2017
Merged

Conversation

samj1912
Copy link
Contributor

No description provided.

@samj1912 samj1912 requested review from mayhem and mineo October 10, 2017 20:16
@samj1912 samj1912 merged commit 1c844a1 into metabrainz:master Oct 12, 2017
@samj1912 samj1912 deleted the solr9 branch October 12, 2017 11:08
@samj1912 samj1912 restored the solr9 branch October 12, 2017 11:08
@mineo
Copy link
Member

mineo commented Oct 12, 2017

I have not tried this, so the following might not be true, but: One important thing to note here is that meta.amazon_asin is not covered by any of the paths of any SearchField of SearchRelease or any of the extrapath entries. This means that its content is not eagerly loaded and you run into the n+1 problem (http://use-the-index-luke.com/sql/join/nested-loops-join-n1-problem, https://secure.phabricator.com/book/phabcontrib/article/n_plus_one/). If you use the --sqltimings option during indexing, you'll see that for every release that's getting processed, one select [...] from release_meta statement for every single release instead of loading them once along with the releases. Adding meta.amazon_asin to extrapaths should fix this. Situations like this are the sole reason extrapaths exists.

@mineo
Copy link
Member

mineo commented Oct 12, 2017

Sorry, the second I was about to close the other tab with the definition of SearchRelease I noticed that it does in fact already load all the asins for the asin field, so ignore the previous comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants