[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Hbase Scan Nodes

["Bikramjeet Vig (Code Review)" <ge...@cloudera.org>]

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 <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 17:50:04 +0000
Gerrit-HasComments: Yes