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