impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-6290: limit ScannerContext to 1 buffer at a time
Date Wed, 10 Jan 2018 19:57:40 GMT
Michael Ho has posted comments on this change. ( )

Change subject: IMPALA-6290: limit ScannerContext to 1 buffer at a time

Patch Set 6:

File be/src/exec/
PS9, Line 195: 
This is not needed because we no longer accumulate completed IO buffers as there is only one
outstanding IO buffer outstanding per stream, right ?
File be/src/exec/scanner-context.h:
PS9, Line 196:     /// Release completed resources, e.g. the last buffer if the current read
position is
nit: one line.
PS9, Line 305:     ///
nit: io_buffer_ is set to null.
File be/src/exec/
PS6, Line 239: if (boundary_buffer_bytes_left_ >= requested_len) break;
> CopyIoToBoundary() copies over some or all of the I/O buffer increments bou
I am still not entirely convinced.

Upon entry of the loop body, the invariant is that boundary_buffer_bytes_left_ + io_buffer_bytes_left_
< requested_len_.

So, even after copying the entire I/O buffer over, boundary_buffer_bytes_left_ should still
be less than requested_len_. So, copying part of the I/O buffer will definitely not make buffer_bytes_left_
>= requested_len_.

I could be missing something but this if-condition seems to be always false and that's what
confuses me.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74c5960a75f7d88b0e1de4199af731fb13e592f0
Gerrit-Change-Number: 8814
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: Zoram Thanga <>
Gerrit-Comment-Date: Wed, 10 Jan 2018 19:57:40 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message