devloopsnet/typesense-php

Consider moving $client to static context

aaronredwood opened this issue · 2 comments

$this->client = new \GuzzleHttp\Client();

I forked your project because I needed to rapidly customize your client and Laravel solution for a private project. Along the way I discovered a performance issue that occurs when using scout:import on a large model (in excess if 1M records). Every couple hundred or so write operations, I would get one of two errors:

  1. A CURL-level error, server timeout, e.g., "
  2. A PHP error about exceeding system limits on open file handles, e.g., "PHP Fatal error: Uncaught ErrorException: fopen(php://stderr): failed to open stream: operation failed in /Users/aaron/repos/c21-access/vendor/symfony/console/Output/ConsoleOutput.php:157"

Both of these problems lead me to debugging the application of Guzzle, ultimately leading to me moving the Guzzle client instantiation inside ApiCall to a static context, like:

private static $client;

protected function getClient()
{
  return self::$client ? self::$client : (self::$client = $this->initClient());
}

protected function initClient()
{
  return new GuzzleHttp\Client();
}

Also worth noting that I downgraded the Guzzle dependency to ^6.3, which is what I require for my Laravel project. Whether or not the above issue with connections and/or file handles is caused by the downgrade isn't something I have time to analyze, but I wasn't able to find any reference to Guzzle specifically causing the above errors.

My dev machine is a Mac (macOS 10.15.6) running PHP 7.3.8.

By the way, thank you very much for this work and for making it open source. I'm sorry that my own project's timeline doesn't lend itself to working more closely with you to improve your own work. I have no intention of releasing our fork publicly, especially given that the Typesense project has selected your codebase to be the official PHP library. I hope that in that change to it being an official library, some steps can be taken to expand the compatibility of the library with older (still supported) versions of Guzzle and older (still supported) versions of PHP. Thank you!

Hey @aaronredwood!

Thank you for all the details you provided to use in order to fix this issue, this repo got officially adopted by typesense today and I'll make sure to apply this fix with the next update.