Adar Dembo has posted comments on this change.

Change subject: Memory tracking for result tracker

Patch Set 9:

File src/kudu/rpc/

Line 75:         mem_tracker_->Consume(sizeof(ClientState));
If we want to make ClientState memory tracking accurate, we need to account for 
two additional things:

1. Malloc "slop". sizeof(ClientState) may not account for the actual size of 
the memory allocation used in "new ClientState()". For example, if the size of 
the struct is 40 bytes, the allocation may be the next power of 2 (i.e. 64 
bytes). We deal with this elsewhere by using kudu_malloc_usable_size(ptr) as 
the "size of object"; sometimes this is internalized inside a 
memory_footprint() method.

2. The size of any nested heap pointers. ClientState contains a map, and that 
map contains nested pointers whose memory consumption isn't being accounted 
for. Look at how Schema::name_to_index_ is managed to see how you might do that.

Line 85:         mem_tracker_->Consume(sizeof(CompletionRecord));
Same issues with CompletionRecord and its nested vector of OngoingRpcs (you can 
use kudu_malloc_usable_size(, though you  need to check that 
vector.capacity() >0 first).

I think you're already taking care of the nested protobuf elsewhere.

Line 234:   mem_tracker_->Consume(response->ByteSize());
Should use SpaceUsed(), I think.

Line 255:       mem_tracker_->Release(sizeof(OnGoingRpcInfo));
We've found that Release() in a loop is generally less efficient than adding up 
all the memory consumed in the loop and making one call to Release() at the 
end. That change reduced block manager CPU consumption pretty dramatically on 
File src/kudu/server/

PS9, Line 89:   if (id != 0) {
            :     StrAppend(&id_str, " ", id);
            :   }
Don't need this; each of these memtrackers is already "disambiguated" by virtue 
of having a unique parent.

That said, why bother creating a memtracker for the ResultTracker instead of 
just reusing the server memtracker? IIRC we only create separate memtrackers 
for objects that come and go (e.g. tablet, memrowset, deltamemstore).

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Jean-Daniel Cryans <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to