Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13266 )

Change subject: WIP [dns_resolver] KUDU-2791 TTL cache in DNS resolver
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13266/3/src/kudu/util/net/dns_resolver.h
File src/kudu/util/net/dns_resolver.h:

http://gerrit.cloudera.org:8080/#/c/13266/3/src/kudu/util/net/dns_resolver.h@73
PS3, Line 73: class AsyncDnsResolver: public DnsResolver {
> I thought it makes sense to have a synchronous-only version.  The idea was
I looked at the implementation and I see what you're saying. But doesn't that 
threadpool thread only spawn if needed? That is, if the synchronous-only 
version _does not_ use the threadpool, there isn't any thread overhead? Nor is 
there any overhead when the pool goes idle and the thread is killed?

Thinking about it some more, I think this would be sufficient:

  // Uses a threadpool with up to N threads to do resolution.
  void ResolveAddressesAsync(const HostPort& hostport,
                             std::vector<Sockaddr>* addresses,
                             const StatusCallback& cb);

  void ResolveAddresses(const HostPort& hostport,
                        std::vector<Sockaddr>* addresses) {
    Synchronizer s;
    ResolveAddressesAsync(hostport, addresses, s.StatusCallback());
    return s.Wait();
  }

When idle, the pool's threads are killed. When active, they're running. Maybe 
we can improve on this, but I don't see why that should mandate a DnsResolver 
subclass; ultimately the interface is the same. We can get clever under the 
hood and not go through the pool for synchronous calls, but again that has no 
bearing on the interface.



--
To view, visit http://gerrit.cloudera.org:8080/13266
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1bbd55a8231fd541d2087f9202f24e80bc79f0b
Gerrit-Change-Number: 13266
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Wed, 29 May 2019 00:15:29 +0000
Gerrit-HasComments: Yes

Reply via email to