Tim Armstrong 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 9:

(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
Yep, that's right. Before this patch ReleaseCompletedResources() freed all of 
the buffers except for the last one (i.e. the previous one returned from 
ReadBytes() or similar). Now buffers are freed as soon as the stream read 
position moves past the end.


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:     /// otherwise returns false and sets 'status'
> nit: one line.
Done


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


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;
> I am still not entirely convinced.
Thanks, i get it now.



--
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: 9
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jan 2018 21:06:27 +0000
Gerrit-HasComments: Yes

Reply via email to