search

parisholley / wordpress-fantastic-elasticsearch

Improve wordpress search performance/accuracy and enable faceted search by leveraging an ElasticSearch server.
MIT License
162 stars 64 forks source link

Increase default of indexer_per_page to 100 #32

Closed michaelsauter closed 11 years ago

michaelsauter commented 11 years ago

The default of 10 is way too low if you have a lot of posts (e.g. thousands) ... and I would argue that you're more likely to use ElasticSearch if you have lots of posts.

parisholley commented 11 years ago

could you also update the tests?

have you tested this using a remote es host like https://searchbox.io/ ? want to make sure that increasing it doesn't cause PHP execution timeouts (typically 30 seconds). This change is probably better off as an option on the admin page and passed in, vs. increasing the default.

michaelsauter commented 11 years ago

Sorry, didn't even think there would be tests for it. I don't think it's good practice to test for specific configuration values.

I haven't tried with searchbox.io, but 100 post should easily be indexed under 30 seconds. That's a long time. If indexing 100 posts would take 30 seconds, then something needs to be tweaked anyhow, because that would be practically unusable.

To me, 100 is much better as a default than 10, as that would need to be changed for basically everyone. With just 10 posts per "page", it takes way too long to index even only 1000 posts.

parisholley commented 11 years ago

Though I agree that testing configuration isn't directly valuable, the better discussion is why is it hard coded :)

Don't forget that the each post has to loop through taxonomies and other potential pieces of data (especially if users extend this plugin with their own behavior), so n+1 is a definite concern here. 100 is just as arbitrary a number as 10 or 1000. I don't see how boosting the page count would improvement performance unless there is a significant cost of initializing wordpress everytime (which is a very real possibility). The code already supports overriding with your own filter, so I don't think this pull request is really needed. The bigger issue is that we should probably be passing the value in the admin when you select the option to re-index.

michaelsauter commented 11 years ago

Ok. I actually thought that the main reason why it's slow is that it does one request per 10 posts. Turns out that it doesn't speed up considerably when using 100 posts per page. Need to look into other ways to make it faster.