Todd Lipcon has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache

Patch Set 1:


didn't do a detailed look at the actual algorithm, mostly some nitty type 
stuff. will need someone else (or me, later) to look at the implementation.

A unit test would be nice
File src/kudu/client/

Line 62: const int MAX_RETURNED_TABLE_LOCATIONS = 10;
doc this
also, should this be in an anonymous namespace? (it's not used outside this 
compilation unit, right?)

PS1, Line 313: ttl:
similar to my comment on the header, this is a bit confusing, because these 
MonoTimes are not human-readable. Either compute a delta from 'now', or try to 
convert to a wall-time?

PS1, Line 612: else
don't need 'else' here or below on 616 because the blocks above return
File src/kudu/client/meta_cache.h:

PS1, Line 257: { }
given there's a default empty constructor defined, might be worth having a 
DCHECK(IsInitialized()) in all the accessors, and implement that either with a 
boolean member or by checking ttl_.IsInitialized()

Line 283:     DCHECK_NOTNULL(tablet_.get());
this will give a warning for an unused result. can just use DCHECK(tablet_);

Line 316:   MonoTime ttl_;
really this is the 'expiration_time_' rather than the 'ttl', right? ie it's a 
time, not a duration

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to