impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <>
Subject [Impala-CR](cdh5-trunk) IMPALA-3611: track unused Disk IO buffer memory
Date Thu, 09 Jun 2016 20:49:24 GMT
Michael Ho has posted comments on this change.

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

Patch Set 4:

File be/src/runtime/

PS4, Line 210: Clear(
Will Reset() be a better name ? This seems to match Reset() below.

PS4, Line 219:   mem_tracker_ = NULL;
nit: Can this be at line 213 so we initialize the fields in the same order as they are declared

Why is it okay to not initialize scan_range_offset_ ? I guess it's always overwritten when
the buffer is consumed, right ?

PS4, Line 648:       // Not a cached buffer. Return the io buffer.
             :       ReturnFreeBuffer(buffer_desc);
nit: should fit in one line.

Line 781:     if (!FLAGS_disable_mem_pools && free_buffers_[idx].size() < FLAGS_max_free_io_buffers)
Long line.

Please also see comments below about free_buffer_mem_tracker.

PS4, Line 856: bool any_io_mgr_limit_exceeded = free_buffer_mem_tracker_->AnyLimitExceeded();
Will it be more obvious if we move this line into ReturnFreeBuffer() instead ? That should
help reduce chances of failure due to exceeding memory limits, right ?

if (!FLAGS_disable_mem_pools &&
    free_buffers_[idx].size() < FLAGS_max_free_io_buffers &&
    free_buffer_mem_tracker_->TryConsume(buffer_size)) {
 . ...
} else {

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I8777cf76f04d34a46f53d53005412e0f1d63b5b7
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message