WebDevStudios / wp-search-with-algolia

Improve search on your site. Autocomplete is included, along with full control over look, feel and relevance.
https://wordpress.org/plugins/wp-search-with-algolia/
146 stars 55 forks source link

2.6.0 causing timeouts/slow indexing #359

Closed SwitchDan closed 1 year ago

SwitchDan commented 1 year ago

Describe the bug We've just upgraded to 2.6.0 from 2.5.4 and when we now save posts or try to Re-Index under Algolia Search > Autocomplete we're often getting timeout issues or generally really slow performance.

We're seeing no errors in the logs but it seems to be something to do with the below file, as if I revert the changes back to 2.5.4 it behaves as expected:

/includes/indices/class-algolia-searchable-posts-index.php

We have a few thousand posts to index on our site, but we haven't changed the default algolia_indexing_batch_size value of 100 (reducing this to 10 doesn't stop timeout issues either).

Screenshots / Video Here's a Loom recording showing the difference in performance between 2.6.0, 2.5.4 and 2.6.0 (HYBRID) which has the file mentioned above reverted to v2.5.4:

https://www.loom.com/share/9800b8dee5b84362b2ca3e11cf894929?sid=6e0e3da4-095e-4402-9f9b-fe8cf984ab1f

Additional context PHP version: 8.0 Wordpress version: 6.3

Hopefully that helps narrow down what the issue might be but let me know if you need any extra info.

tw2113 commented 1 year ago

Some quick initial notes

Here are the changes made to that file between the versions: https://github.com/WebDevStudios/wp-search-with-algolia/compare/2.5.4...2.6.0#diff-e8d55fcbd771f2559ad1a6d5342151de0676f966ccd323b3b319c8dfc0f31a1a

Here's the issue that prompted the change: https://github.com/WebDevStudios/wp-search-with-algolia/issues/327 Overall pull request: https://github.com/WebDevStudios/wp-search-with-algolia/pull/328/files

tw2113 commented 1 year ago

@asharirfan What are your thoughts on amending the changes for https://github.com/WebDevStudios/wp-search-with-algolia/pull/328 to have it not default to true but does have a filter so that the original person could still make use of the wait() change.

colis commented 1 year ago

@tw2113 @asharirfan I reckon the wait operation should be disabled during batch re-indexing, since posts are gonna be deleted anyway and the order of operations is not relevant.

The class variable $this->reindexing declared in class-algolia-index.php should be available to the child classes class-algolia-searchable-posts-index.php and class-algolia-posts-index.php so its value could be used to either enable or disable the wait() operation, e.g.

if ( ! empty( $records ) ) {
    $this->delete_item( $post, ! $this->reindexing );
}

I'm happy to open a PR if you're not already working on it.

SwitchDan commented 1 year ago

Thanks for looking into this guys.

I just want to add that we have slowdowns when saving posts as well - it seems to save the post and then start the indexing process and you're stuck around waiting for it to complete before the screen reloads which really slows down editing on sites with lots of content.

A filter would be a nice option for us personally. We've not encountered any of the indexing issues that prompted the updates in #328 so we'd be fine with just disabling entirely if we can.

tw2113 commented 1 year ago

We're going to keep it simple, though thank you for the PR offer @colis

You can see my proposed changes for this over at https://github.com/WebDevStudios/wp-search-with-algolia/compare/fix/359-performance-issues-with-wait?expand=1

We will offer a filter in place of the 2nd parameter on delete_item() and have it default to false. Changing that behavior can be as simple as a one liner like so:

add_filter( 'algolia_should_wait_on_delete_item', '__return_true' ); 

That said, it'll default to false to return majority of the users to asynchronous indexing.

tw2113 commented 1 year ago

Closing as released. All reports I've seen have been positive so I feel good with closing this.