queueRemoveFromSearch() should optionally use a queue

[chamby]

Engine delete operations, much like update operations, invoke the network and add latency. I would expect Searchable::queueRemoveFromSearch() to optionally use a queue just like Searchable::queueMakeSearchable() does. It even has queue in the name.

A new Job called MakeUnsearchable would also need to be created.

[nunomaduro]

I kind of agree with you on this one. Possible code for a Pull Request:

    /**
     * Dispatch the job to make the given models unsearchable.
     *
     * @param  \Illuminate\Database\Eloquent\Collection  $models
     * @return void
     */
    public function queueRemoveFromSearch($models)
    {
        if ($models->isEmpty()) {
            return;
        }

        if (! config('scout.queue')) {
            return $models->first()->searchableUsing()->delete($models);
        }

        dispatch((new RemoveFromSearch($models))
                ->onQueue($models->first()->syncWithSearchUsingQueue())
                ->onConnection($models->first()->syncWithSearchUsing()));
    }

@driesvints What do you think about this?

[driesvints]

@nunomaduro looks good.

[nunomaduro]

Thanks @driesvints. I gonna work on a Pull Request on the next couple days.

Edit: I don't really have time for this right now.

[michaelnguyen2021]

I just found out today that unsearchable() does not use queue. This PR will fix my issue. Thanks

[mvdnbrk]

I am trying to implement this with the code suggested by @nunomaduro.

We will dispatch a job RemoveFromSearch passing $models which is an instance of \Illuminate\Database\Eloquent\Collection, but by the time the job will be executed the models are already deleted from the database.

Any suggestions how to get around this?

[jshayes]

I think that it why queueing deletes was initially removed in this commit f46b2b8

[chamby]

Yes. This limitation makes sense. It might be possible to obtain the relevant keys for deletion using $model->getScoutKey() and queue the RemoveFromSearch job with that data instead of attempting to de-serialize the model itself in the Job. The Scout keys are all that is actually needed issue a deletion request (* See my edit below).

Knowing which Scout engine to use for the model class being deleted may also need to be persisted if that information isn't easily available at the job's execution time.

*Edit: After looking at the Laravel\Scout\Engines\Engine interface some modifications would have to be made there as well. Right now the delete() method requires a Collection of model instances. The Scout keys will not be enough. This interface would need to be expanded and that may be asking too much for this functionality.

[nathanblogs]

Any update on this, It was really confusing looking at the code to find out it doesn't actually queue.

[jasonlfunk]

This bit me today. I'm using the TNTSearch driver and have a very large search index. Even though I have a dedicated scout queue, this method is being run in sync has causing 4-5s delays in my code. This really ought to be given a higher priority.

[driesvints]

Anyone's free to send in a pr for this enhancement.

[matt-allan]

I have this working on a branch but it's a bit of a hack.

It only works if the scout driver only accesses $model->getScoutKey; any other attributes will be null. This works fine with the Algolia driver. The Elasticsearch driver I'm personally using accesses other model attributes to determine the document type so it wouldn't work.

Given these limitations I'm not sure if I should open a PR. Maybe it should be optional?

[driesvints]

@matt-allan I think it's fair to pursue a solution for only the Algolia driver since that's the only one we're supporting natively in the library atm. But if there would be a generic solution to this we can definitely improve it further later on so other third party libraries can benefit as well.

[chamby]

@matt-allan I just wanted to ping you about your branch. I think you should submit a PR for the Algolia support. It seems like you have enough buy-in.

[matt-allan]

Thanks @chamby and @driesvints, I'll see if I can make some time to get the PR ready next week.

[driesvints]

@matt-allan I've tackled the remaining issues. Feel free to send that PR for this one if you like! :)

[fernandocoronatomf]

Hey guys, any idea about when this will be released?
I think this definitely should run in a queue when the scout driver is set to true.

[driesvints]

@fernandocoronatomf please don't post "when will this be released" comments. Those aren't helpful. Instead, feel free to help out by sending in a PR.

[uyson]

2 year ?

[driesvints]

@uyson please send in a PR if you want this to move forward.

[uyson]

yes, I tring to do . but I need to learn phpunit, and mockry to write a test

[uyson]

我看了代码， 原来的基础场不好改， 搜两年了我很郁闷官方也没有修改的想法？

[driesvints]

This will finally land in today's Scout release thanks to @stevebauman 👏