Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14129 )
Change subject: IMPALA-8819: BufferedPlanRootSink should handle non-default fetch sizes ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.h File be/src/exec/buffered-plan-root-sink.h: http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.h@85 PS2, Line 85: 1024 QueryState::DEFAULT_BATCH_SIZE ? http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.h@136 PS2, Line 136: = nullptr; nit: unnecessary for unique_ptr http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.h@144 PS2, Line 144: /// 'current_batch' points to. This needs to be called with 'lock_' held right ? http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.h@145 PS2, Line 145: bool IsQueueEmpty() { bool IsQueueEmpty const { http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.cc File be/src/exec/buffered-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.cc@143 PS2, Line 143: // If 'num_results' <= 0 then by default fetch BATCH_SIZE rows. > I did some profiling, and I think the answer is that it depends on the netw Thanks for doing the experiment. 10x BATCH_SIZE seems like a reasonable starting point. http://gerrit.cloudera.org:8080/#/c/14129/2/be/src/exec/buffered-plan-root-sink.cc@162 PS2, Line 162: if (current_batch_ == nullptr) if (!current_batch_) http://www.cplusplus.com/reference/memory/unique_ptr/operator%20bool/ http://gerrit.cloudera.org:8080/#/c/14129/2/tests/query_test/test_result_spooling.py File tests/query_test/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/14129/2/tests/query_test/test_result_spooling.py@264 PS2, Line 264: for _ in range(int(ceil(float(self._num_rows) / float(fetch_size)))): Given the assert at line 268, why not just do: while rows_fetched < self._num_rows: -- To view, visit http://gerrit.cloudera.org:8080/14129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8dd4b397ab6457a4f85e635f239b2c67130fcce4 Gerrit-Change-Number: 14129 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Sat, 24 Aug 2019 01:08:07 +0000 Gerrit-HasComments: Yes
