jolicode / elastically

🔍 JoliCode's Elastica wrapper to bootstrap Elasticsearch PHP integrations
248 stars 37 forks source link

Throw an exception when the Indexer queue is not cleared #42

Open damienalexandre opened 3 years ago

damienalexandre commented 3 years ago

This is kind of experimental but I had this idea where we can prevent developers from misusing the Indexer.

Upon __destruct, we check the Indexer queue, and if there is Documents here we throw an exception :boom:.

My only fear is that:

Attempting to throw an exception from a destructor (called in the time of script termination) causes a fatal error.

Is it an issue?

This PR also adds a "clear" method to empty the queue.

jdreesen commented 3 years ago

Why not flush the queue on destruct, instead of throwing an exception?

damienalexandre commented 3 years ago

Yeah that's also possible. I'm open to suggestions :+1:

renanbr commented 3 years ago

Hello you all,

I'm not sure requesting a database or throwing an exception in the destructor is a good thing.

IMO people should rely on framework triggers to handle it. As this lib is Symfony friendly, we could add an event listener to do something in the kernel.finish_request or kernel.terminate for example.

jdreesen commented 3 years ago

I'm not sure requesting a database or throwing an exception in the destructor is a good thing.

You're right. I'm doing this in my current project (which uses another elasticsearch libaray):

    public function __destruct()
    {
        if ($this->bulkCount > 0) {
            $this->commit();
        }
    }

It mostly (99% of the time) runs fine, but sometimes it throws like this at the end:

Fatal error: Uncaught ErrorException: Warning: curl_setopt_array(): supplied resource is not a valid cURL handle resource in /var/www/html/vendor/guzzlehttp/ringphp/src/Client/CurlFactory.php on line 52

Call Stack:
    0.0002     392512   1. {main}() /var/www/html/bin/console:0
    8.6368   83180968   2. MyApp\Console\Application->run() /var/www/html/bin/console:26

ErrorException: Warning: curl_setopt_array(): supplied resource is not a valid cURL handle resource in /var/www/html/vendor/guzzlehttp/ringphp/src/Client/CurlFactory.php on line 52

Call Stack:
    0.0002     392512   1. {main}() /var/www/html/bin/console:0
    8.6368   83180968   2. MyApp\Console\Application->run() /var/www/html/bin/console:26
   21.3915  103627104   3. AppBundle\Search\DocumentRepository\DocumentRepository->__destruct() /var/www/html/src/AppBundle/Search/DocumentRepository/DocumentRepository.php:0
   21.3915  103627104   4. AppBundle\Search\DocumentRepository\DocumentRepository->commit() /var/www/html/src/AppBundle/Search/DocumentRepository/DocumentRepository.php:29
   21.3915  103627104   5. ONGR\ElasticsearchBundle\Service\IndexService->commit() /var/www/html/src/AppBundle/Search/DocumentRepository/DocumentRepository.php:64
   21.3915  103627480   6. Elasticsearch\Client->bulk() /var/www/html/vendor/ongr/elasticsearch-bundle/Service/IndexService.php:455
   21.3918  103640088   7. Elasticsearch\Client->performRequest() /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Client.php:792
   21.3918  103640128   8. Elasticsearch\Transport->performRequest() /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Client.php:1586
   21.3918  103640128   9. Elasticsearch\Connections\Connection->performRequest() /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Transport.php:107
   21.3918  103641256  10. Elasticsearch\Connections\Connection->Elasticsearch\Connections\{closure:/var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php:212-318}() /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php:187
   21.3918  103632776  11. GuzzleHttp\Ring\Client\Middleware::GuzzleHttp\Ring\Client\{closure:/var/www/html/vendor/guzzlehttp/ringphp/src/Client/Middleware.php:28-32}() /var/www/html/vendor/elasticsearch/elasticsearch/src/Elasticsearch/Connections/Connection.php:218
   21.3918  103632776  12. GuzzleHttp\Ring\Client\CurlHandler->__invoke() /var/www/html/vendor/guzzlehttp/ringphp/src/Client/Middleware.php:30
   21.3918  103632872  13. GuzzleHttp\Ring\Client\CurlHandler->_invokeAsArray() /var/www/html/vendor/guzzlehttp/ringphp/src/Client/CurlHandler.php:68
   21.3918  103632872  14. GuzzleHttp\Ring\Client\CurlFactory->__invoke() /var/www/html/vendor/guzzlehttp/ringphp/src/Client/CurlHandler.php:84
   21.3919  103635696  15. curl_setopt_array() /var/www/html/vendor/guzzlehttp/ringphp/src/Client/CurlFactory.php:52
   21.3919  103636744  16. Symfony\Component\Debug\ErrorHandler->handleError() /var/www/html/vendor/guzzlehttp/ringphp/src/Client/CurlFactory.php:52

So it's probably not a good idea to do this...

jdreesen commented 3 years ago

Why not flush the queue on destruct, instead of throwing an exception?

Instead of flushing on __destruct() we could flush via register_shutdown_function(), too. I'm not very familiar with these functions, I just stumbled upon it and thought it may be an option.