Adar Dembo has posted comments on this change.

Change subject: Memory tracking for result tracker
......................................................................


Patch Set 21: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3627/19/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 109:       request_id.seq_no(),
> isn't that handled by the 'mem_Tracker_->Consume(client_state->memory_footp
Ah, missed that since it didn't change.

So the old code was:
1. Potentially adjust consumption of result tracker if a client state was added 
(as that would change the value of client_state_map_bytes).
1. Consume for the new client state.
2. Potentially adjust consumption of client state if a completion record is 
added (as that would change the value of completion_record_map_bytes_).
3. Consume for the new completion record.
4. Potentially adjust consumption of completion record if an ongoing RPC is 
added (as that would change the ongoing_rpcs vector allocation).

Since client_state_map_bytes_ and completion_record_map_bytes_ have been 
replaced with MemTrackerAllocators, you can safely remove step #1 and #3, which 
were implemented by the two removed ScopedMemTrackerUpdaters. Is that right?


Line 296: 
> the ClientState's memory usage is already released through an explicit ->Re
Right, we're not actually freeing an entire ClientState down this path; this 
was just here to handle changes to ClientState consumption due to a removed 
completion record. And now you've taken care of that with the replacement of 
client_state_map_bytes_.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to