Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11306 )
Change subject: IMPALA-7351: Improve memory estimates for Hbase Scan Nodes ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/exec/hbase-table-scanner.cc File be/src/exec/hbase-table-scanner.cc: http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/exec/hbase-table-scanner.cc@107 PS1, Line 107: value_pool_(new MemPool(scan_node_->mem_tracker(),true)), > whitespace Done http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/runtime/mem-pool.h File be/src/runtime/mem-pool.h: http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/runtime/mem-pool.h@55 PS1, Line 55: enforce_binary_chunk_sizes > "binary" feels like a non-obvious way to describe it. Maybe "po2" instead? yea I struggled with the name a bit for the fear of it being too verbose. A few other options I had though of: - exponential - exp2 - pow2 - pow_of_2 Any favorites? http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/runtime/mem-pool.cc File be/src/runtime/mem-pool.cc: http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/runtime/mem-pool.cc@132 PS1, Line 132: if(enforce_binary_chunk_sizes_) chunk_size = BitUtil::RoundUpToPowerOfTwo(chunk_size); > whitespace Done http://gerrit.cloudera.org:8080/#/c/11306/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java File fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java: http://gerrit.cloudera.org:8080/#/c/11306/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@511 PS1, Line 511: public void computeNodeResourceProfile(TQueryOptions queryOptions) { > This function feels slightly too large - it's hard to understand the calcul Done http://gerrit.cloudera.org:8080/#/c/11306/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@517 PS1, Line 517: // allocate-clear cycle on mem-pool for each row fetched from hbase. To get an approx > Can you cross-reference from the be code? Done http://gerrit.cloudera.org:8080/#/c/11306/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@565 PS1, Line 565: mem_estiamte > typo Done -- To view, visit http://gerrit.cloudera.org:8080/11306 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583545c3f5e454854f111871c5fbc4f108ae4bff Gerrit-Change-Number: 11306 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 13 Sep 2018 17:50:04 +0000 Gerrit-HasComments: Yes
