Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/17195 )
Change subject: IMPALA-10584: Defer advancing read page when reservation is tight. ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/17195/4/be/src/runtime/buffered-tuple-stream.cc File be/src/runtime/buffered-tuple-stream.cc: http://gerrit.cloudera.org:8080/#/c/17195/4/be/src/runtime/buffered-tuple-stream.cc@733 PS4, Line 733: read_it_.read_page_ == pages_.begin() > I wonder if this assertion is always true, since in pinned mode, there can This DCHECK was part of my debugging when investigating the bug. The intent is to match assertion in NextReadPage (buffered-tuple-stream.cc:529). Agree that this DCHECK most likely will always be true since we check earlier that attached_to_output_batch == true. Will remove this DCHECK in next patch set. http://gerrit.cloudera.org:8080/#/c/17195/4/be/src/runtime/buffered-tuple-stream.cc@750 PS4, Line 750: unpin this first page > May add a comment on when this currently unpinned page is finally unpinned. Ack -- To view, visit http://gerrit.cloudera.org:8080/17195 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I16137b6e423f190f60c3115a06ccd0f77e9f585a Gerrit-Change-Number: 17195 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Tue, 13 Apr 2021 20:06:19 +0000 Gerrit-HasComments: Yes
