Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cooperatively resolve hosts to avoid running same query concurrently #125

Merged
merged 1 commit into from Apr 6, 2019

Conversation

clue
Copy link
Member

@clue clue commented Apr 1, 2019

This PR adds a new CoopExecutor which cooperatively resolves hosts via the given base executor to ensure same query is not run concurrently.

This is useful because all executors are entirely async and as such allow you to execute any number of queries concurrently. You should probably limit the number of concurrent queries in your application or you're very likely going to face rate limitations and bans on the resolver end. For many common applications, you may want to avoid sending the same query multiple times when the first one is still pending, so you will likely want to use this in combination with some other executor (which is the new default).

This is best reproduced by using some higher-level protocol implementation, for example it's rather common to send hundreds of (RESTful) HTTP API calls to the same host name, often concurrently. In this case, it doesn't make much sense to send the same DNS query multiple times.

Note that this is not to be confused with caching (which is already implemented to some degree). This PR only deals with avoiding concurrency while caching deals with queries that happen after a query is already completed.

Here's a synthetic example (99-excessive.php reactphp.org 1000) which highlights this low-level API. Benchmarking suggests this example improved from ~390ms down to ~160ms using a local DNS resolver (YMMV).

<?php

use React\Dns\Config\Config;
use React\Dns\Resolver\Factory;

require __DIR__ . '/../vendor/autoload.php';

$loop = React\EventLoop\Factory::create();

$config = Config::loadSystemConfigBlocking();
$server = $config->nameservers ? reset($config->nameservers) : '8.8.8.8';

$factory = new Factory();
$resolver = $factory->create($server, $loop);

if (!isset($argv[2])) {
    echo 'Usage: ' . $argv[0] . ' <domain> <n>' . PHP_EOL;
    exit(1);
}

$domain = $argv[1];
$n = (int)$argv[2];

echo 'go…';
$ok = $failed = 0;
for ($i = 0; $i < $n; ++$i) {
    $resolver->resolve($domain)->then(function () use (&$ok, &$failed, $n) {
        ++$ok;
        echo "\r" . $ok . '/' . $failed . '/' . $n;
    }, function () use (&$ok, &$failed, $n) {
        ++$failed;
        echo "\r" . $ok . '/' . $failed . '/' . $n;
    });
}

$loop->run();

echo "\r" . $ok . '/' . $failed . '/' . $n . ' done'. PHP_EOL;

Closes #90

Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 🇮🇹

a `CoopExecutor` like this:

```php
$executor = new CoopExecutor(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe RequestSharingExecutor / QuerySharingExecutor is a better name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. I don't have a strong preference for either name, so I'll leave this up to somebody else to decide if somebody has a strong preference and/or some good arguments 👍

@jsor jsor merged commit 2192705 into reactphp:master Apr 6, 2019
@clue clue deleted the coop branch April 7, 2019 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit number of concurrent DNS queries
4 participants