impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
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. ( http://gerrit.cloudera.org:8080/8814 )

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


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8814/9/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/8814/9/be/src/exec/hdfs-scanner.cc@a195
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 ?


http://gerrit.cloudera.org:8080/#/c/8814/9/be/src/exec/scanner-context.h
File be/src/exec/scanner-context.h:

http://gerrit.cloudera.org:8080/#/c/8814/9/be/src/exec/scanner-context.h@196
PS9, Line 196:     /// Release completed resources, e.g. the last buffer if the current read
position is
nit: one line.


http://gerrit.cloudera.org:8080/#/c/8814/9/be/src/exec/scanner-context.h@305
PS9, Line 305:     ///
nit: io_buffer_ is set to null.


http://gerrit.cloudera.org:8080/#/c/8814/6/be/src/exec/scanner-context.cc
File be/src/exec/scanner-context.cc:

http://gerrit.cloudera.org:8080/#/c/8814/6/be/src/exec/scanner-context.cc@239
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 http://gerrit.cloudera.org:8080/8814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

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 <tarmstrong@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zoram@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jan 2018 19:57:40 +0000
Gerrit-HasComments: Yes

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