Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11947 )

Change subject: IMPALA-6656: BufferAllocator observability
......................................................................


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11947/4/be/src/runtime/bufferpool/buffer-allocator.cc
File be/src/runtime/bufferpool/buffer-allocator.cc:

http://gerrit.cloudera.org:8080/#/c/11947/4/be/src/runtime/bufferpool/buffer-allocator.cc@37
PS4, Line 37: //
> nit: /// Same below.
I don't think we usually use triple slash for constants. I checked a couple of 
places.


http://gerrit.cloudera.org:8080/#/c/11947/4/be/src/runtime/bufferpool/buffer-pool-test.cc
File be/src/runtime/bufferpool/buffer-pool-test.cc:

http://gerrit.cloudera.org:8080/#/c/11947/4/be/src/runtime/bufferpool/buffer-pool-test.cc@75
PS4, Line 75: test_env_->DisableBufferPool();
> Can you please add a comment on why this is necessary ?
Done


http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json@1330
PS4, Line 1330: allocating from this system
> in system allocator ?
Done


http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json@1340
PS4, Line 1340: Counts the number of times
> "Number of times" may be more succinct. Same below.
Done


http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json@1350
PS4, Line 1350:  allocated
> directly allocated
Done


http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json@1350
PS4, Line 1350: the number
> the number of times
Done


http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json@1370
PS4, Line 1370: a free buffer was recycled within same NUMA node to fulfi
> a recycled buffer within the same NUMA node was used to fufil... ?
Done



--
To view, visit http://gerrit.cloudera.org:8080/11947
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18
Gerrit-Change-Number: 11947
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 23:10:52 +0000
Gerrit-HasComments: Yes

Reply via email to