matchish / laravel-scout-elasticsearch

Search among multiple models with ElasticSearch and Laravel Scout
MIT License
702 stars 113 forks source link

Adaptive import option #280

Closed DavisPuciriuss closed 2 months ago

DavisPuciriuss commented 3 months ago

Based on some performance testing (here), I have added an 'adaptive' import option. That will check if parallel import is 'worth it' in that case.

Useful when having multiple imports going, with varying document count. This is still a work in progress, would like your opinion if this is even something that should be added, or should just be kept under the same '--parallel' option.

Things to do: Check Queue::size of the parallel-import queues, if they are full or currently handling another import - disable parallel import. More performance testing, as the number of documents where parallel import is faster than regular could vary.

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Please upload report for BASE (8.x@304ff4a). Learn more about missing BASE report.

Files Patch % Lines
src/Jobs/ImportStages.php 87.50% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## 8.x #280 +/- ## ====================================== Coverage ? 98.01% Complexity ? 259 ====================================== Files ? 40 Lines ? 808 Branches ? 0 ====================================== Hits ? 792 Misses ? 16 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

matchish commented 2 months ago

Not sure about it. A dev could decide itself by running with or without parallel. I mean speed could depends on setup like machine number of parallel queues, IO performance. So 75000 it's for your setup

DavisPuciriuss commented 2 months ago

Closing this PR for now, as the performance is highly dependent on the machines hardware specs.