Typesense Import: Index Race Condition

[saibotk]

Scout Version

10.9.0

Scout Driver

Typesense

Laravel Version

10.48.10

PHP Version

8.3.9

Database Driver & Version

Typesense 0.25.2

SDK Version

Typesense PHP 4.9.3

Meilisearch CLI Version

No response

Description

When deleting the index and afterwards importing all models, we can reach a race condition, where the import fails because some jobs run in parallel and all try to create or access the index.

Only one succeeds and the others will, for example, throw this error:
Typesense\Exceptions\ObjectNotFound: Not Found
or interestingly, only rarely, Typesense\Exceptions\ObjectAlreadyExists: A collection with name already exists..

This happens when using queued imports. I don't exactly understand why it returns Not Found instead of the more rare collection already exists error, which is logical, but this might be some technical detail in the typesense engine.

E.g. using Horizon with minProcesses = 5 and running the following command on a table with enough entries e.g. ~ 40.000 will sometimes result in those exceptions on some of the jobs.

We should try to make the import more robust and maybe ensure that the index is created before the documents are imported? I'm happy to hear your solutions on this, but i think we should fix this flaw in scout directly.

Steps To Reproduce

In any scout repository with Horizon or some queue workers and a table with enough elements such that chunking happens, run:

php artisan scout:delete-index "App\Models\User" && php artisan scout:import "App\Models\User"

And sometimes the race condition will cause some jobs to fail.

[driesvints]

@jasonbosco @AbdullahFaqeir

[saibotk]

Seems like #822 is related

[saibotk]

I just did some more deep-diving:

• scout:flush in typesense does also delete the schema / index entirely on the server, but the local exists bool is never reset to null, so it will assume that it exists. This is one of the issues that we need to solve. I think this is the cause of the ObjectNotFound exception. I found this by using flush instead of delete-index.
• the other one still needs investigation, maybe there are different code paths and some weird race conditions on what horizon process has which state etc. idk

A small workaround:
When calling artisan scout:flush or scout:delete-index run this afterwards (maybe create a wrapper artisan command):

use Laravel\Scout\EngineManager;
use App\Models\Commodity;

$ts = app(EngineManager::class)->engine();
$model = new Commodity();

$ts->flush($model);
$ts->getCollections()->offsetUnset($model->searchableAs());

Note: This will only happen in queue workers, where i suppose the engine is kept alive? because the process is not restarted after a job?
PS: Yes, so this is mainly a caching / state management issue, due to the collection exists cache:

[driesvints]

Thanks a lot for your investigation @saibotk. If you're up for it, definitely feel free to send in a PR with a proposed change.

[jasonbosco]

@saibotk thank you for digging into this. Let me know if you're planning to do a PR for this.

If not, @tharropoulos from the Typesense team is also available to look into this.

[saibotk]

I could try to come up with some fix for this, but am currently pretty swamped.
Also I think it is necessary to see how this might need some broader change. Because currently i don't know how the cached exist came to be and in which way we need to preserve the functionality of that caching.

Obviously we could just drop the cache and always do an exists query but im also not sure if there might be some better solution using other typesense methods.

Im open to discuss approaches and would appreciate any insights

[tharropoulos]

When I tried to take a look at the server logs, if you use queue:work, these requests are sent:

If you then restart the worker and execute the import once again, these requests are sent to Typesense:

If you then use queue:listen, these requests are sent, and the jobs don't fail.

It must be applicable to the static state generated when flushing the collection, since the change is reflected on the queue:listen command. I'm still looking into it, as I'm not sure what's the best way to approach this would be. I'm not sure we should drop the cache as @saibotk already mentioned.

EDIT: provided some more info regarding the differences between queue:listen and queue:work

[tharropoulos]

The getOrCreateCollectionFromModel method in TypesenseEngine is not calling out to the API, having its state of collections as a single source of truth:

scout/src/Engines/TypesenseEngine.php

Lines 497 to 505 in 3174219

	protected function getOrCreateCollectionFromModel($model, bool $indexOperation = true): TypesenseCollection
	{
	$method = $indexOperation ? 'indexableAs' : 'searchableAs';
	$collection = $this->typesense->getCollections()->{$model->{$method}()};
	if ($collection->exists() === true) {
	return $collection;
	}

To ensure matching the state from the server to the worker, if a collection is found, it should then call out to the Typesense server to see if the collection exists on the server as well, if not, it will have to create it on the server.

    protected function getOrCreateCollectionFromModel($model, bool $indexOperation = true): TypesenseCollection
    {
        $method = $indexOperation ? 'indexableAs' : 'searchableAs';

        $collection = $this->typesense->getCollections()->{$model->{$method}()};

        // Initialize a flag to check if the collection exists
        $collectionExists = false;
        if ($collection->exists()) {
            $collectionName = $model->{$method}();
            try  {
                $this->typesense->collections[$collectionName]->retrieve();
                $collectionExists = true;
            } catch (TypesenseClientError $e){
                $collectionExists = false;
            }
        }

        // Use the flag to determine the next steps
        if ($collectionExists) {
            return $this->typesense->getCollections()->{$collectionName};
        }
...

[tharropoulos]

@saibotk First of all, thank you for taking the time to investigate so thoroughly and getting us in the right track. Could you verify that this change fixes the problems on your side as well?

EDIT: fix typo

[saibotk]

Yes i will do so on friday when im back at work thanks for taking the time 🙌

[saibotk]

@tharropoulos Okay got time to test this now and thats my result:

• Importing after flush now works and is not dependent on the static state in the worker, thats nice!
• If you quickly repeat a "flush && import" command you can still get it to print a message that collection X does not exist (even though it does not throw an error) That's correct because flush is immediate and thus a job might try to post a request to the import endpoint when the collection was just deleted, so nothing wrong here!
• Also I think there might be some edge cases where this still fails or rather just tries to create a collection even though it already exists (if it is set to false), but maybe that is fine by design?
• With this change i don't know why we now even bother keeping track of the "exists" variable. It seems like this state is only used inside of the getOrCreate function and no where else. It could even be deleted entirely from this repository and the typesense php sdk too, since we now check if it actually exists anyway.

What do you think?

Thank you for the quick response!