Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3611: track unused Disk IO buffer memory
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3799/6/be/src/runtime/disk-io-mgr.cc
File be/src/runtime/disk-io-mgr.cc:

Line 364:   }
> this is fine but when do we construct without calling Init()?
If ExecEnv::StartServices() exits early this can happen.

I think also in some backend tests we will construct the ExecEnv without 
calling StartServices()


http://gerrit.cloudera.org:8080/#/c/3799/6/be/src/runtime/disk-io-mgr.h
File be/src/runtime/disk-io-mgr.h:

Line 280:     bool is_cached_;
> when is this not equivalent to scan_range_->cached_buffer_ != NULL?  Or is 
The second. In HandleReadFinished() we call EnqueueBuffer(), which can remove 
the scan_range_ from the bfufer. I'm realising though that in general it's not 
valid to call any buffer methods after calling EnqueueBuffer(), so instead of 
adding this member I should just call is_cached() before EnqueueBuffer() and 
save the value.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8777cf76f04d34a46f53d53005412e0f1d63b5b7
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to