Swader / diffbot-php-client

[Deprecated - Maintenance mode - use APIs directly please!] The official Diffbot client library
MIT License
53 stars 20 forks source link

Diffbot client not closing TCP connections until client exits #58

Open jonathantullett opened 6 years ago

jonathantullett commented 6 years ago

When making a call to create or update a job, the client is not closing TCP connections after the request to the api endpoint.

Running the following code demonstrates the issue:

    $diffbot = new Diffbot("xxxxxxxxx");
    $job = $diffbot->crawl("jonathan_test");
    $job->setSeeds(["http://www.example.com"])->setMaxToCrawl(100)
        ->setMaxToProcess(100)->setMaxRounds(1)
        ->setOnlyProcessIfNew(1)->setMaxHops(3);

    $api
        = $diffbot->createArticleAPI('crawl')->setMeta(true)->setDiscussion(false)
        ->setQuerystring(true)
    ;

    $job->setApi($api);
    $x = $job->call();
sleep(100);

Now the socket remains open until the process quits:

$ netstat -an|  grep 443                                                                                                       
tcp        0      0 192.168.22.214:50844    35.192.184.37:443       TIME_WAIT

While not too bad for a single socket, if you create a lot of diffbot objects using new Diffbot(), you can quickly run out of open files on the system as the sockets aren't closed even when the object falls out of scope.

jonathantullett commented 6 years ago

I wrote a wrapper for the Diffbot client which implemented a singleton and this has resolved the issue for me.

<?php
namespace DDA;
class Diffbot
{
    private static $instance;
    private function __construct() {}
    private static $apiKey = "sadffddsfafafdfdsdfsdfs";
    /**
     * @return \Swader\Diffbot\Diffbot
     * @throws \Zend_Exception
     */
    public static function getInstance(): \Swader\Diffbot\Diffbot
    {
        if(self::$instance === null) {
            self::$instance = new \Swader\Diffbot\Diffbot(self::$apiKey);
        }
        return self::$instance;
    }
}

So I would propose that rather than calling new Diffbot(),each time you call the wrapped static method getInstance() which returns the singleton instance.

While the root cause - dangling TCP connections - is still there, it's only a single one, from the single Diffbot object, being re-used extensively, even when updating thousands of searches.

I would propose a similar approach is taken within your library.