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

Sync only on Entry Events #205

Closed
toyflish opened this issue Aug 23, 2021 · 20 comments
Closed

Sync only on Entry Events #205

toyflish opened this issue Aug 23, 2021 · 20 comments
Assignees

Comments

@toyflish
Copy link

I got a Section with huge Entries with a matrix with hundreds of fields in 4 languages. I have another Section Routines that are indexed in algolia.
With scout-plugin enabled when I hit save on an entry in the huge-entry-section, the request takes 2minutes to resolve with hundreds of sql selects going on.
When I disable scout-plugin, the time to save huge entries goes down to 5 seconds.

I guess this is because scout listens to Elements::EVENT_AFTER_SAVE_ELEMENT on every field on the huge entry and ramps up hundreds of sql-selects to check if they fit the indexing-criteria.

How can I remove this behaviour and just have scout to action on Entry::EVENT_AFTER_SAVE so that it just adds one more query for criteria-matching to saving an entry instead of hundreds?

this is my scout.php creating four indices for four languages:

<?php
use modules\algoliamodule\services\AlgoliaModuleService;
use \rias\scout\ScoutIndex;
use \rias\scout\IndexSettings;

$INDEX_LOCALES = ['en', 'de', 'fr', 'nl'];

$reoutineIndex  = function($localeKey) {
    return ScoutIndex::create('routine_' . $localeKey)
        ->elementType(\craft\elements\Entry::class)
        ->criteria(function (\craft\elements\db\EntryQuery $query) use ($localeKey) {
            return $query->section('routine')->site($localeKey);
        })
        ->transformer(function (\craft\elements\Entry $entry) use ($localeKey) {
            return (new AlgoliaModuleService)->transformRoutine($entry, $localeKey); 
        })
        ->indexSettings( IndexSettings::create()
            ->minWordSizefor1Typo(4)
            ->attributesForFaceting([
                0 => 'routineTopics',
                1 => 'painPoints',
                2 => 'sports',
                3 => 'trainingGoals',
            ])
        );
};

return [
    'sync' => true,
    'queue' => true,
    'connect_timeout' => 3,
    'batch_size' => 100,
    'application_id' => getenv('ALGOLIA_APPLICATION_ID'),
    'admin_api_key'  => getenv('ALGOLIA_ADMIN_API_KEY'),
    'search_api_key' => getenv('ALGOLIA_SEARCH_API_KEY'), //optional
    'indices' => array_merge(array_map($reoutineIndex, $INDEX_LOCALES)),
];
@janhenckens
Copy link
Member

Hey @toyflish, good point, I can see how that would create an issue.
Maybe moving the events here:

craft-scout/src/Scout.php

Lines 142 to 146 in e7709b6

$events = [
[Elements::class, Elements::EVENT_AFTER_SAVE_ELEMENT],
[Elements::class, Elements::EVENT_AFTER_RESTORE_ELEMENT],
[Elements::class, Elements::EVENT_AFTER_UPDATE_SLUG_AND_URI],
];

To the settings/config file with these as default. That way you could change them out as needed.

Seems like a pretty safe change.

@sjcallender
Copy link

@toyflish - We're seeing the same thing, but it seems it only began after recently updating to Craft 3.7.x. Is that your experience too?

@janhenckens - We're eager for a solution too. If necessary, I believe we can resort to console sync on a cron, but something that allows us to keep content-based syncing from the CP would be phenomenal.

@janhenckens
Copy link
Member

@sjcallender Hmm, weird that it would only show up after 3.7.x, the plugin certainly hasn't changed since then and Craft's internal events we're listening for have always been Element based, triggering for entries as well as matrix blocks.

I'll try to work out the above solution and get back to you asap!

@timkelty
Copy link
Collaborator

timkelty commented Sep 2, 2021

@sjcallender can you make sure you're on the latest (craftcms/cms@3.7.11), we just fixed an index regression issue that affected save performance.

@sjcallender
Copy link

@timkelty - Thanks. We're on 3.7.11 after Brad and Oli noted it in a support email. It didn't help in our case. Only disabling Scout helps.

@timkelty
Copy link
Collaborator

timkelty commented Sep 2, 2021

@sjcallender 👍 Just talked it over with @janhenckens and I think he has a plan to address.

janhenckens added a commit that referenced this issue Sep 2, 2021
janhenckens added a commit that referenced this issue Sep 2, 2021
@janhenckens
Copy link
Member

@toyflish @sjcallender Just released 2.4.2 which includes a new ShouldBeSearchableEvent, that should give you the option to bail out of the searchable behavior for anything that's not an entry. Docs can be found here. Could you give this solution a try and report back?

Kudos to @timkelty for the back and forth :)

@sjcallender
Copy link

Sweet. Thanks @janhenckens.

My main back-end dev is on vacation and I'm not a PHP/plugin dev, so what file would that snippet from the docs go into? Into a custom module?

@janhenckens
Copy link
Member

Yes, in a module. You'd have to add some code to check the element in question too, just adding it like the example will result in nothing ever being searchable again so you don't want that :)

@toyflish
Copy link
Author

toyflish commented Sep 6, 2021

thanks @janhenckens,
when I save the page and check with is_a($event->element, 'craft\elements\Entry') for having a Entry-element, I get the event fired for all entries linked to the current Entry and all other entries in that structure (not 100% sure but looks like from the logs).
How can I access the entry that is currently edited in the cp from inside that event-hook to only return true if thats the one the event is fired for?

@sjcallender
Copy link

sjcallender commented Sep 8, 2021

@janhenckens - I'm a bit confused on how to implement the event too.

We have a homepage entry (id=2) that, currently, when saved, triggers a sync for all its related product entries. We want to stop syncing these products when the homepage entry is saved. We added the following to our module.

Event::on(
            SearchableBehavior::class,
            SearchableBehavior::EVENT_SHOULD_BE_SEARCHABLE,
            function (ShouldBeSearchableEvent $event) {

                if ($event->element->getId() === 2) {
                    $event->shouldBeSearchable = false;
                }
            }
        );

The above still syncs the products.

However, if change the conditional to the following, we're no longer syncing.

                if ($event->element->getId() !== 2) {
                    $event->shouldBeSearchable = false;
                }

As you can guess, this second conditional blocks all entries from being synced even when they are saved directly.

How should we be setting up our logic to prevent syncing of related entries only?

@sjcallender
Copy link

Got it working using the slug.

        // prevent syncing related entries 
        Event::on(
            SearchableBehavior::class,
            SearchableBehavior::EVENT_SHOULD_BE_SEARCHABLE,
            function (ShouldBeSearchableEvent $event) {
                // after saving homepage entry
                if ($event->element->slug === 'homepage') {
                    $event->shouldBeSearchable = false;
                }
            }
        );

@janhenckens
Copy link
Member

thanks @janhenckens,
when I save the page and check with is_a($event->element, 'craft\elements\Entry') for having a Entry-element, I get the event fired for all entries linked to the current Entry and all other entries in that structure (not 100% sure but looks like from the logs).
How can I access the entry that is currently edited in the cp from inside that event-hook to only return true if thats the one the event is fired for?

Hmm, fair point @toyflish. Sounds like you want to disable the updating of related events then?

@sjcallender
Copy link

only return true if thats the one the event is fired for

@janhenckens - That's what we're trying to do too.

In my code above, I'm realizing it works only if I'm not "applying" a draft, provisional or regular. Some guidance would be appreciated.

@janhenckens
Copy link
Member

@sjcallender @toyflish Sounds like not having related entries automatically indexed would fix it for you both?

I had intended the event to give you the option to check wether or not the element is an entry and not for example a matrix block, like it @toyflish's original question. I'll have a think on how to check for related entries.

@heymarkreeves
Copy link

@janhenckens Weighing in to also request disabling automatic indexing of related entries. We, too, have a very large database and would prefer to just push the entry being saved into Algolia for performance reasons. Related content will be pushed as those entries are saved.

davist11 pushed a commit to vigetlabs/craft-scout that referenced this issue Oct 1, 2021
davist11 pushed a commit to vigetlabs/craft-scout that referenced this issue Oct 1, 2021
@janhenckens
Copy link
Member

Turns out Mr. @timkelty has done this already in #175 :). Would a config setting to simply disable syncing of related elements outright work in this case?

@heymarkreeves
Copy link

Would a config setting to simply disable syncing of related elements outright work in this case?

@janhenckens I think so, yes. We don't have any scenarios where we want to sync the related elements when syncing any one particular element. Related content would be indexed within that element's document, or indexed independently when it was saved. Thanks!

@janhenckens
Copy link
Member

Just released 2.5.0 which has this included. You can now set indexRelations to false in your scout config.

Could you give this a try and report back if this improves things for you @heymarkreeves @sjcallender @toyflish? I'll leave this issue open for a bit, feel free to jump in with your thoughts!

@sjcallender
Copy link

@janhenckens It works great! Thanks for this.

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

No branches or pull requests

5 participants