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

Change subject: WIP KUDU-2791: TTL cache in DNS resolver (part 1)
......................................................................


Patch Set 4:

(3 comments)

All of the builds failed with:

18:31:52 [ 69%] Linking CXX shared library 
../../../lib/exported/libkudu_client.so
18:31:52 /usr/bin/ld: 
../../../../../thirdparty/installed/uninstrumented/lib/libmemkind.a(memkind.o): 
relocation R_X86_64_32 against `.data' can not be used when making a shared 
object; recompile with -fPIC
18:31:52 ../../../../../thirdparty/installed/uninstrumented/lib/libmemkind.a: 
error adding symbols: Bad value

This suggests that we aren't building memkind with -fPIC. We should either do 
that, or make it impossible for the memkind symbols to show up in the exported 
client library. Unfortunately this change introduces that link by adding 
references to Cache inside DnsResolver.

http://gerrit.cloudera.org:8080/#/c/13266/4/src/kudu/util/net/dns_resolver-test.cc
File src/kudu/util/net/dns_resolver-test.cc:

http://gerrit.cloudera.org:8080/#/c/13266/4/src/kudu/util/net/dns_resolver-test.cc@88
PS4, Line 88:   LOG(INFO) << Substitute("$0 non-cached resolutions of '$1' took 
$2",
Are 1000 iterations of non-cached DNS lookup fast enough for this test?


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

http://gerrit.cloudera.org:8080/#/c/13266/4/src/kudu/util/net/dns_resolver.h@82
PS4, Line 82:   typedef TTLCache<std::string, std::vector<Sockaddr>> 
HostRecordCache;
Should doc what the keys and values are.


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

http://gerrit.cloudera.org:8080/#/c/13266/4/src/kudu/util/net/dns_resolver.cc@105
PS4, Line 105: resolved_addresses
Should use cached_addresses here for consistency.

But I think the more accurate way to account for the charge here is:

  kudu_malloc_usable_size(cached_addresses) + cached_addresses.capacity() > 0 ? 
kudu_malloc_usable_size(cached_addresses.data()) : 0;



--
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: 4
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Thu, 30 May 2019 03:00:48 +0000
Gerrit-HasComments: Yes

Reply via email to