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

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


Patch Set 2:

(1 comment)

> client_symbols-test is failing:
 >
 > Found bad symbol 'vmem_aligned_alloc'
 > Found bad symbol 'vmem_calloc'
 > Found bad symbol 'vmem_check'
 > Found bad symbol 'vmem_check_version'
 > Found bad symbol 'vmem_create'
 > Found bad symbol 'vmem_create_in_region'
 > Found bad symbol 'vmem_delete'
 > Found bad symbol 'vmem_errormsg'
 > Found bad symbol 'vmem_free'
 > Found bad symbol 'vmem_malloc'
 > Found bad symbol 'vmem_malloc_usable_size'
 > Found bad symbol 'vmem_realloc'
 > Found bad symbol 'vmem_set_funcs'
 > Found bad symbol 'vmem_stats_print'
 > Found bad symbol 'vmem_strdup'
 > Kudu client library contains 15 bad symbols
 > 
 > You have two choices:
 > 1. Do some additional refactoring to convince the linker that even
 > though you're creating a TTLCache which, under the hood, could
 > create an NVM cache, you'll never actually do that, and it
 > shouldn't bring the vmem symbols into the client library.
 > 2. Punt and declare these symbols as local in client/symbols.map.
 >
 > I'd prefer #1 so as to reduce the size of the client library, but
 > if it's not a huge size difference and if it proves to be
 > challenging, maybe #2 is OK.

Thank you for taking a look at the failures.  I also found that after a couple 
of attempts to restart Jenkins build job.  However, it required me to run the 
tests locally using ctest, since dist-test didn't report any issues: 
http://dist-test.cloudera.org//job?job_id=aserbin.1557248586.130621

Right, it seems it's necessary to do something with nvmcache-related stuff.  
Probably, I'll address it later: I didn't expect it to be a non-trivial change 
like that.

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

http://gerrit.cloudera.org:8080/#/c/13266/2/src/kudu/util/net/dns_resolver.h@28
PS2, Line 28: #include "kudu/util/ttl_cache.h"
> Could you forward declare TTLCache instead? Or does that not work because o
I relied on IWYU suggestion here.  But it's possible to have it 
forward-declared, as I understand, yes.



--
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: 2
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, 08 May 2019 03:48:48 +0000
Gerrit-HasComments: Yes

Reply via email to