[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. IMPALA-6656: BufferAllocator observability Adds a set of metrics per allocator arena in the buffer pool that help understand how buffers are being allocated and how much time is spent in the system allocator (i.e. TCMalloc). These are low level metrics that require some interpretation but provide visibility into behaviour that was previously totally opaque. Also tracks the total time spent in the system allocator in the query profile, to provide clues if time spent in TCMalloc is a perf issue for a particular query (e.g. if it's hitting a lot of lock contention). Backend tests required tweaks to avoid double-registration of the new metrics. Also switch default sort in /metrics to be by name, so that it's easier to locate metrics. Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 Reviewed-on: http://gerrit.cloudera.org:8080/11947 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/runtime/bufferpool/buffer-allocator-test.cc M be/src/runtime/bufferpool/buffer-allocator.cc M be/src/runtime/bufferpool/buffer-allocator.h M be/src/runtime/bufferpool/buffer-pool-counters.h M be/src/runtime/bufferpool/buffer-pool-test.cc M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/bufferpool/suballocator-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/test-env.cc M be/src/runtime/test-env.h M be/src/util/metrics.h M common/thrift/metrics.json M www/metric_group.tmpl 14 files changed, 274 insertions(+), 72 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- 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: merged Gerrit-Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 Gerrit-Change-Number: 11947 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 6: Verified+1 -- 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: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 01 Dec 2018 04:51:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 6: Code-Review+2 -- 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: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 01 Dec 2018 00:56:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3510/ DRY_RUN=false -- 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: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 01 Dec 2018 00:56:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 5: Code-Review+2 -- 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: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 01 Dec 2018 00:31:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1486/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 30 Nov 2018 23:44:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 5: Code-Review+1 carry +1 -- 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: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 30 Nov 2018 23:11:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 30 Nov 2018 23:10:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Hello Michael Ho, Zoltan Borok-Nagy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11947 to look at the new patch set (#5). Change subject: IMPALA-6656: BufferAllocator observability .. IMPALA-6656: BufferAllocator observability Adds a set of metrics per allocator arena in the buffer pool that help understand how buffers are being allocated and how much time is spent in the system allocator (i.e. TCMalloc). These are low level metrics that require some interpretation but provide visibility into behaviour that was previously totally opaque. Also tracks the total time spent in the system allocator in the query profile, to provide clues if time spent in TCMalloc is a perf issue for a particular query (e.g. if it's hitting a lot of lock contention). Backend tests required tweaks to avoid double-registration of the new metrics. Also switch default sort in /metrics to be by name, so that it's easier to locate metrics. Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 --- M be/src/runtime/bufferpool/buffer-allocator-test.cc M be/src/runtime/bufferpool/buffer-allocator.cc M be/src/runtime/bufferpool/buffer-allocator.h M be/src/runtime/bufferpool/buffer-pool-counters.h M be/src/runtime/bufferpool/buffer-pool-test.cc M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/bufferpool/suballocator-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/test-env.cc M be/src/runtime/test-env.h M be/src/util/metrics.h M common/thrift/metrics.json M www/metric_group.tmpl 14 files changed, 274 insertions(+), 72 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/11947/5 -- 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: newpatchset Gerrit-Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 Gerrit-Change-Number: 11947 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Michael Ho 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. 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 ? 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 ? 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. http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json@1350 PS4, Line 1350: the number the number of times http://gerrit.cloudera.org:8080/#/c/11947/4/common/thrift/metrics.json@1350 PS4, Line 1350: allocated directly allocated 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... ? -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 30 Nov 2018 22:10:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1453/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 28 Nov 2018 20:45:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 4: Code-Review+1 -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 28 Nov 2018 20:10:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Hello Michael Ho, Zoltan Borok-Nagy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11947 to look at the new patch set (#4). Change subject: IMPALA-6656: BufferAllocator observability .. IMPALA-6656: BufferAllocator observability Adds a set of metrics per allocator arena in the buffer pool that help understand how buffers are being allocated and how much time is spent in the system allocator (i.e. TCMalloc). These are low level metrics that require some interpretation but provide visibility into behaviour that was previously totally opaque. Also tracks the total time spent in the system allocator in the query profile, to provide clues if time spent in TCMalloc is a perf issue for a particular query (e.g. if it's hitting a lot of lock contention). Backend tests required tweaks to avoid double-registration of the new metrics. Also switch default sort in /metrics to be by name, so that it's easier to locate metrics. Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 --- M be/src/runtime/bufferpool/buffer-allocator-test.cc M be/src/runtime/bufferpool/buffer-allocator.cc M be/src/runtime/bufferpool/buffer-allocator.h M be/src/runtime/bufferpool/buffer-pool-counters.h M be/src/runtime/bufferpool/buffer-pool-test.cc M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/bufferpool/suballocator-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/test-env.cc M be/src/runtime/test-env.h M be/src/util/metrics.h M common/thrift/metrics.json M www/metric_group.tmpl 14 files changed, 272 insertions(+), 72 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/11947/4 -- 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: newpatchset Gerrit-Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 Gerrit-Change-Number: 11947 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 3: Code-Review+1 (2 comments) carry http://gerrit.cloudera.org:8080/#/c/11947/3/be/src/runtime/bufferpool/buffer-allocator.cc File be/src/runtime/bufferpool/buffer-allocator.cc: http://gerrit.cloudera.org:8080/#/c/11947/3/be/src/runtime/bufferpool/buffer-allocator.cc@38 PS3, Line 38: // various useful statistics that would be too expensive to update every allocation. > nit: Maybe it's worth to mention that we should keep it as a power-of-2, so Done http://gerrit.cloudera.org:8080/#/c/11947/3/be/src/runtime/bufferpool/buffer-allocator.cc@367 PS3, Line 367: int64_t sys_alloc_time = sys_alloc_sw.ElapsedTime(); > Shouldn't we move this line up by one line to only measure the allocation t 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: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 28 Nov 2018 20:06:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 3: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/11947/3/be/src/runtime/bufferpool/buffer-allocator.cc File be/src/runtime/bufferpool/buffer-allocator.cc: http://gerrit.cloudera.org:8080/#/c/11947/3/be/src/runtime/bufferpool/buffer-allocator.cc@38 PS3, Line 38: // various useful statistics that would be too expensive to update every allocation. nit: Maybe it's worth to mention that we should keep it as a power-of-2, so the calculation of the modulo remain more efficient. http://gerrit.cloudera.org:8080/#/c/11947/3/be/src/runtime/bufferpool/buffer-allocator.cc@367 PS3, Line 367: int64_t sys_alloc_time = sys_alloc_sw.ElapsedTime(); Shouldn't we move this line up by one line to only measure the allocation time? -- 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: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 28 Nov 2018 15:30:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1400/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 19 Nov 2018 20:38:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11947 to look at the new patch set (#3). Change subject: IMPALA-6656: BufferAllocator observability .. IMPALA-6656: BufferAllocator observability Adds a set of metrics per allocator arena in the buffer pool that help understand how buffers are being allocated and how much time is spent in the system allocator (i.e. TCMalloc). These are low level metrics that require some interpretation but provide visibility into behaviour that was previously totally opaque. Also tracks the total time spent in the system allocator in the query profile, to provide clues if time spent in TCMalloc is a perf issue for a particular query (e.g. if it's hitting a lot of lock contention). Backend tests required tweaks to avoid double-registration of the new metrics. Also switch default sort in /metrics to be by name, so that it's easier to locate metrics. Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 --- M be/src/runtime/bufferpool/buffer-allocator-test.cc M be/src/runtime/bufferpool/buffer-allocator.cc M be/src/runtime/bufferpool/buffer-allocator.h M be/src/runtime/bufferpool/buffer-pool-counters.h M be/src/runtime/bufferpool/buffer-pool-test.cc M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/bufferpool/suballocator-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/test-env.cc M be/src/runtime/test-env.h M be/src/util/metrics.h M common/thrift/metrics.json M www/metric_group.tmpl 14 files changed, 271 insertions(+), 72 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/11947/3 -- 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: newpatchset Gerrit-Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 Gerrit-Change-Number: 11947 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1395/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 19 Nov 2018 19:54:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11947/2/be/src/runtime/bufferpool/buffer-pool.cc File be/src/runtime/bufferpool/buffer-pool.cc: http://gerrit.cloudera.org:8080/#/c/11947/2/be/src/runtime/bufferpool/buffer-pool.cc@115 PS2, Line 115: // TODO: register metrics Need to remove TODO -- 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: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 19 Nov 2018 18:59:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6656: BufferAllocator observability
Tim Armstrong has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11947 ) Change subject: IMPALA-6656: BufferAllocator observability .. IMPALA-6656: BufferAllocator observability Adds a set of metrics per allocator arena in the buffer pool that help understand how buffers are being allocated and how much time is spent in the system allocator (i.e. TCMalloc). These are low level metrics that require some interpretation but provide visibility into behaviour that was previously totally opaque. Also tracks the total time spent in the system allocator in the query profile, to provide clues if time spent in TCMalloc is a perf issue for a particular query (e.g. if it's hitting a lot of lock contention). Backend tests required tweaks to avoid double-registration of the new metrics. Also switch default sort in /metrics to be by name, so that it's easier to locate metrics. Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 --- M be/src/runtime/bufferpool/buffer-allocator-test.cc M be/src/runtime/bufferpool/buffer-allocator.cc M be/src/runtime/bufferpool/buffer-allocator.h M be/src/runtime/bufferpool/buffer-pool-counters.h M be/src/runtime/bufferpool/buffer-pool-test.cc M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/bufferpool/suballocator-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/test-env.cc M be/src/runtime/test-env.h M be/src/util/metrics.h M common/thrift/metrics.json M www/metric_group.tmpl 14 files changed, 272 insertions(+), 72 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/11947/2 -- 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: newpatchset Gerrit-Change-Id: I12b740b8ea7773b3215681531dfa62db55cfdf18 Gerrit-Change-Number: 11947 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong
