You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2018/11/21 12:20:05 UTC
[kudu-CR] [tools] Add tool to dump memtrackers
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11975
Change subject: [tools] Add tool to dump memtrackers
......................................................................
[tools] Add tool to dump memtrackers
This adds a new tool, `kudu diagnose dump_mem_trackers`, that dumps the
mem-trackers information. It contains information equivalent to the
tracker table on /mem-trackers. There's a new RPC introduced to support
this tool. Output both as JSON and as a table is supported. The table
output type flattens the tracker hierarchy, but it can be
reconstructed from the 'id' and 'parent_id' columns.
Some Kudu community members have reported to me they have started using
the /mem-trackers page to investigate performance and memory issues with
Kudu clusters, and having a way to get the same information in a form
amenable to programmatic searching, sorting, and filtering is
convenient. This tool can also serve as a starting point for more
automated analysis of the tracker information.
The particular advantage of the memtracker dump over, say, the data
generated by heap sampling is that the memtracker shows where memory is
owned. The sampling just shows where it was allocated in a call stack.
So, for example, using the memtracker information it's possible to tell
very quickly exactly which tablets are using a lot of memory in
DeltaMemStores. Heap samples would indicate memory allocated in
update-related code paths, but it would not be possible to tell which
tablet replicas actually held the memory that was allocated.
Here's a small sample of output in csv table format:
tablet-2e02d00d46834b359c6ba0f8d471cbf9,server,-1,2318,2583
txn_tracker,tablet-2e02d00d46834b359c6ba0f8d471cbf9,67108864,0,0
MemRowSet-1,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
DeltaMemStores,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
tablet-2d40450f6556485eab92f64f90756b61,server,-1,2318,2583
txn_tracker,tablet-2d40450f6556485eab92f64f90756b61,67108864,0,0
MemRowSet-1,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
DeltaMemStores,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
The columns are id,parent-id,limit,current_consumption,peak_consumption
just like the table on /mem-trackers.
Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
---
M src/kudu/server/generic_service.cc
M src/kudu/server/generic_service.h
M src/kudu/server/server_base.proto
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_diagnose.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
A src/kudu/util/mem_tracker.proto
10 files changed, 322 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/11975/1
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Mitch Barnett, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11975
to look at the new patch set (#5).
Change subject: [tools] Add tool to dump memtrackers
......................................................................
[tools] Add tool to dump memtrackers
This adds a new tool, `kudu diagnose dump_mem_trackers`, that dumps the
mem-trackers information. It contains information equivalent to the
tracker table on /mem-trackers. There's a new RPC introduced to support
this tool. Output both as JSON and as a table is supported. The table
output type flattens the tracker hierarchy, but it can be
reconstructed from the 'id' and 'parent_id' columns.
Some Kudu community members have reported to me they have started using
the /mem-trackers page to investigate performance and memory issues with
Kudu clusters, and having a way to get the same information in a form
amenable to programmatic searching, sorting, and filtering is
convenient. This tool can also serve as a starting point for more
automated analysis of the tracker information.
The particular advantage of the memtracker dump over, say, the data
generated by heap sampling is that the memtracker shows where memory is
owned. The sampling just shows where it was allocated in a call stack.
So, for example, using the memtracker information it's possible to tell
very quickly exactly which tablets are using a lot of memory in
DeltaMemStores. Heap samples would indicate memory allocated in
update-related code paths, but it would not be possible to tell which
tablet replicas actually held the memory that was allocated.
Here's a small sample of output in csv table format:
tablet-2e02d00d46834b359c6ba0f8d471cbf9,server,-1,2318,2583
txn_tracker,tablet-2e02d00d46834b359c6ba0f8d471cbf9,67108864,0,0
MemRowSet-1,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
DeltaMemStores,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
tablet-2d40450f6556485eab92f64f90756b61,server,-1,2318,2583
txn_tracker,tablet-2d40450f6556485eab92f64f90756b61,67108864,0,0
MemRowSet-1,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
DeltaMemStores,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
The columns are id,parent-id,limit,current_consumption,peak_consumption
just like the table on /mem-trackers.
Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
---
M src/kudu/server/generic_service.cc
M src/kudu/server/generic_service.h
M src/kudu/server/server_base.proto
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_diagnose.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
A src/kudu/util/mem_tracker.proto
15 files changed, 411 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/11975/5
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
Patch Set 1:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/server/generic_service.cc@259
PS1, Line 259: const auto root = server_->mem_tracker();
const auto*
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/server/generic_service.cc@260
PS1, Line 260: if (root) {
Isn't this something we can assume?
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/kudu-admin-test.cc@2060
PS1, Line 2060: TEST_F(AdminCliTest, TestDumpMemTrackers) {
Should also update kudu-tool-test's "help" tests.
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc
File src/kudu/tools/tool_action_diagnose.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc@47
PS1, Line 47: DECLARE_int64(timeout_ms);
Shouldn't this be exposed via AddOptionalParameter() too?
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc@49
PS1, Line 49: DEFINE_string(output, "table",
It's a little funky to describe the format option here. Why not override the description for format using an AddOptionalParameter override?
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc@155
PS1, Line 155: If the port is "
: "omitted, the default tablet server port is used"
So historically we've duplicated these kinds of tools into 'master' and 'tserver' in order to get the right default port for each. The actual tool body goes into tool_action_common.cc so the code isn't actually duplicated.
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker-test.cc
File src/kudu/util/mem_tracker-test.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker-test.cc@129
PS1, Line 129:
Maybe also tickle the consumption/peak_consumption values a bit?
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.cc
File src/kudu/util/mem_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.cc@201
PS1, Line 201: auto& tracker_pb = tracker_and_pb.second;
Not auto* ?
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto
File src/kudu/util/mem_tracker.proto:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@19
PS1, Line 19: package kudu;
Should probably also set a java_package.
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@22
PS1, Line 22: // Note that recursive message are not a good idea in general because
messages
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@24
PS1, Line 24: a maximum
: // depth of 3
This isn't enforced by memtrackers so it could conceivably change in the future.
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@29
PS1, Line 29: required string id = 1;
I think our guidance is to avoid introducing new 'required' fields, even if they're required by the application.
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Nov 2018 16:48:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Mitch Barnett, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11975
to look at the new patch set (#4).
Change subject: [tools] Add tool to dump memtrackers
......................................................................
[tools] Add tool to dump memtrackers
This adds a new tool, `kudu diagnose dump_mem_trackers`, that dumps the
mem-trackers information. It contains information equivalent to the
tracker table on /mem-trackers. There's a new RPC introduced to support
this tool. Output both as JSON and as a table is supported. The table
output type flattens the tracker hierarchy, but it can be
reconstructed from the 'id' and 'parent_id' columns.
Some Kudu community members have reported to me they have started using
the /mem-trackers page to investigate performance and memory issues with
Kudu clusters, and having a way to get the same information in a form
amenable to programmatic searching, sorting, and filtering is
convenient. This tool can also serve as a starting point for more
automated analysis of the tracker information.
The particular advantage of the memtracker dump over, say, the data
generated by heap sampling is that the memtracker shows where memory is
owned. The sampling just shows where it was allocated in a call stack.
So, for example, using the memtracker information it's possible to tell
very quickly exactly which tablets are using a lot of memory in
DeltaMemStores. Heap samples would indicate memory allocated in
update-related code paths, but it would not be possible to tell which
tablet replicas actually held the memory that was allocated.
Here's a small sample of output in csv table format:
tablet-2e02d00d46834b359c6ba0f8d471cbf9,server,-1,2318,2583
txn_tracker,tablet-2e02d00d46834b359c6ba0f8d471cbf9,67108864,0,0
MemRowSet-1,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
DeltaMemStores,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
tablet-2d40450f6556485eab92f64f90756b61,server,-1,2318,2583
txn_tracker,tablet-2d40450f6556485eab92f64f90756b61,67108864,0,0
MemRowSet-1,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
DeltaMemStores,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
The columns are id,parent-id,limit,current_consumption,peak_consumption
just like the table on /mem-trackers.
Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
---
M src/kudu/server/generic_service.cc
M src/kudu/server/generic_service.h
M src/kudu/server/server_base.proto
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_diagnose.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
A src/kudu/util/mem_tracker.proto
15 files changed, 412 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/11975/4
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
[tools] Add tool to dump memtrackers
This adds a new tool, `kudu diagnose dump_mem_trackers`, that dumps the
mem-trackers information. It contains information equivalent to the
tracker table on /mem-trackers. There's a new RPC introduced to support
this tool. Output both as JSON and as a table is supported. The table
output type flattens the tracker hierarchy, but it can be
reconstructed from the 'id' and 'parent_id' columns.
Some Kudu community members have reported to me they have started using
the /mem-trackers page to investigate performance and memory issues with
Kudu clusters, and having a way to get the same information in a form
amenable to programmatic searching, sorting, and filtering is
convenient. This tool can also serve as a starting point for more
automated analysis of the tracker information.
The particular advantage of the memtracker dump over, say, the data
generated by heap sampling is that the memtracker shows where memory is
owned. The sampling just shows where it was allocated in a call stack.
So, for example, using the memtracker information it's possible to tell
very quickly exactly which tablets are using a lot of memory in
DeltaMemStores. Heap samples would indicate memory allocated in
update-related code paths, but it would not be possible to tell which
tablet replicas actually held the memory that was allocated.
Here's a small sample of output in csv table format:
tablet-2e02d00d46834b359c6ba0f8d471cbf9,server,-1,2318,2583
txn_tracker,tablet-2e02d00d46834b359c6ba0f8d471cbf9,67108864,0,0
MemRowSet-1,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
DeltaMemStores,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
tablet-2d40450f6556485eab92f64f90756b61,server,-1,2318,2583
txn_tracker,tablet-2d40450f6556485eab92f64f90756b61,67108864,0,0
MemRowSet-1,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
DeltaMemStores,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
The columns are id,parent-id,limit,current_consumption,peak_consumption
just like the table on /mem-trackers.
Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Reviewed-on: http://gerrit.cloudera.org:8080/11975
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/server/generic_service.cc
M src/kudu/server/generic_service.h
M src/kudu/server/server_base.proto
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_diagnose.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
A src/kudu/util/mem_tracker.proto
15 files changed, 411 insertions(+), 12 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Mitch Barnett, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11975
to look at the new patch set (#3).
Change subject: [tools] Add tool to dump memtrackers
......................................................................
[tools] Add tool to dump memtrackers
This adds a new tool, `kudu diagnose dump_mem_trackers`, that dumps the
mem-trackers information. It contains information equivalent to the
tracker table on /mem-trackers. There's a new RPC introduced to support
this tool. Output both as JSON and as a table is supported. The table
output type flattens the tracker hierarchy, but it can be
reconstructed from the 'id' and 'parent_id' columns.
Some Kudu community members have reported to me they have started using
the /mem-trackers page to investigate performance and memory issues with
Kudu clusters, and having a way to get the same information in a form
amenable to programmatic searching, sorting, and filtering is
convenient. This tool can also serve as a starting point for more
automated analysis of the tracker information.
The particular advantage of the memtracker dump over, say, the data
generated by heap sampling is that the memtracker shows where memory is
owned. The sampling just shows where it was allocated in a call stack.
So, for example, using the memtracker information it's possible to tell
very quickly exactly which tablets are using a lot of memory in
DeltaMemStores. Heap samples would indicate memory allocated in
update-related code paths, but it would not be possible to tell which
tablet replicas actually held the memory that was allocated.
Here's a small sample of output in csv table format:
tablet-2e02d00d46834b359c6ba0f8d471cbf9,server,-1,2318,2583
txn_tracker,tablet-2e02d00d46834b359c6ba0f8d471cbf9,67108864,0,0
MemRowSet-1,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
DeltaMemStores,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
tablet-2d40450f6556485eab92f64f90756b61,server,-1,2318,2583
txn_tracker,tablet-2d40450f6556485eab92f64f90756b61,67108864,0,0
MemRowSet-1,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
DeltaMemStores,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
The columns are id,parent-id,limit,current_consumption,peak_consumption
just like the table on /mem-trackers.
Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
---
M src/kudu/server/generic_service.cc
M src/kudu/server/generic_service.h
M src/kudu/server/server_base.proto
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_diagnose.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
A src/kudu/util/mem_tracker.proto
15 files changed, 412 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/11975/3
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_common.h
File src/kudu/tools/tool_action_common.h:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_common.h@136
PS3, Line 136: Status DumpMemTrackers(const std::string& address, uint16_t default_port);
> Doc.
Done
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_diagnose.cc
File src/kudu/tools/tool_action_diagnose.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_diagnose.cc@44
PS3, Line 44: using std::array;
> warning: using decl 'array' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_diagnose.cc@45
PS3, Line 45: using std::cout;
> warning: using decl 'cout' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_diagnose.cc@46
PS3, Line 46: using std::endl;
> warning: using decl 'endl' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_master.cc@169
PS3, Line 169: .AddOptionalParameter("format")
> Did you need to override the description for this? And in tool_action_tserv
No, I think the description for 'memtracker_output' covers it:
"One of 'json', 'json_compact' or 'table'. Table output flattens "
"the memtracker hierarchy."
--format kicks in when 'table' output is chosen.
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_tserver.cc
File src/kudu/tools/tool_action_tserver.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_tserver.cc@166
PS3, Line 166: tablet server
> Nit: Kudu Tablet Server
Done
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/util/mem_tracker-test.cc
File src/kudu/util/mem_tracker-test.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/util/mem_tracker-test.cc@192
PS3, Line 192: gc000->Consume(40);
: gc000->Release(20);
> On second thought, I think we want to promote usage of SCOPED_CLEANUP over
Done
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/util/mem_tracker-test.cc@192
PS3, Line 192: gc000->Consume(40);
: gc000->Release(20);
> You can use ScopedTrackedConsumption here to avoid the SCOPED_CLEANUP below
N/A
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 17:45:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
Patch Set 1:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/server/generic_service.cc@259
PS1, Line 259: const auto root = server_->mem_tracker();
> const auto*
N/A if we don't need to check 'root'.
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/server/generic_service.cc@260
PS1, Line 260: if (root) {
> Isn't this something we can assume?
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/kudu-admin-test.cc@2060
PS1, Line 2060: TEST_F(AdminCliTest, TestDumpMemTrackers) {
> Should also update kudu-tool-test's "help" tests.
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc
File src/kudu/tools/tool_action_diagnose.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc@47
PS1, Line 47: DECLARE_int64(timeout_ms);
> Shouldn't this be exposed via AddOptionalParameter() too?
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc@49
PS1, Line 49: DEFINE_string(output, "table",
> It's a little funky to describe the format option here. Why not override th
Good idea. I forget that I can do that.
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/tools/tool_action_diagnose.cc@155
PS1, Line 155: If the port is "
: "omitted, the default tablet server port is used"
> So historically we've duplicated these kinds of tools into 'master' and 'ts
OK. I kinda think that's icky: any tool that operates against a master or a tablet server must be listed under 'master' and 'tserver', regardless of the character of the tool itself. This tool in particular is a niche diagnostic tool that happens to work against both kinds of servers but probably has 99% of its utility with tablet servers. On the other hand it is nice not to have to remember the port.
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker-test.cc
File src/kudu/util/mem_tracker-test.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker-test.cc@129
PS1, Line 129:
> Maybe also tickle the consumption/peak_consumption values a bit?
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.cc
File src/kudu/util/mem_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.cc@198
PS1, Line 198: to_process.top()
> nit: maybe, might save a bit on copying if using std::move(to_process.top()
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.cc@201
PS1, Line 201: auto& tracker_pb = tracker_and_pb.second;
> Not auto* ?
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto
File src/kudu/util/mem_tracker.proto:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@19
PS1, Line 19: package kudu;
> Should probably also set a java_package.
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@22
PS1, Line 22: // Note that recursive message are not a good idea in general because
> messages
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@24
PS1, Line 24: a maximum
: // depth of 3
> This isn't enforced by memtrackers so it could conceivably change in the fu
Done
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.proto@29
PS1, Line 29: required string id = 1;
> I think our guidance is to avoid introducing new 'required' fields, even if
Done
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 23:06:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Mitch Barnett, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11975
to look at the new patch set (#2).
Change subject: [tools] Add tool to dump memtrackers
......................................................................
[tools] Add tool to dump memtrackers
This adds a new tool, `kudu diagnose dump_mem_trackers`, that dumps the
mem-trackers information. It contains information equivalent to the
tracker table on /mem-trackers. There's a new RPC introduced to support
this tool. Output both as JSON and as a table is supported. The table
output type flattens the tracker hierarchy, but it can be
reconstructed from the 'id' and 'parent_id' columns.
Some Kudu community members have reported to me they have started using
the /mem-trackers page to investigate performance and memory issues with
Kudu clusters, and having a way to get the same information in a form
amenable to programmatic searching, sorting, and filtering is
convenient. This tool can also serve as a starting point for more
automated analysis of the tracker information.
The particular advantage of the memtracker dump over, say, the data
generated by heap sampling is that the memtracker shows where memory is
owned. The sampling just shows where it was allocated in a call stack.
So, for example, using the memtracker information it's possible to tell
very quickly exactly which tablets are using a lot of memory in
DeltaMemStores. Heap samples would indicate memory allocated in
update-related code paths, but it would not be possible to tell which
tablet replicas actually held the memory that was allocated.
Here's a small sample of output in csv table format:
tablet-2e02d00d46834b359c6ba0f8d471cbf9,server,-1,2318,2583
txn_tracker,tablet-2e02d00d46834b359c6ba0f8d471cbf9,67108864,0,0
MemRowSet-1,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
DeltaMemStores,tablet-2e02d00d46834b359c6ba0f8d471cbf9,-1,265,265
tablet-2d40450f6556485eab92f64f90756b61,server,-1,2318,2583
txn_tracker,tablet-2d40450f6556485eab92f64f90756b61,67108864,0,0
MemRowSet-1,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
DeltaMemStores,tablet-2d40450f6556485eab92f64f90756b61,-1,265,265
The columns are id,parent-id,limit,current_consumption,peak_consumption
just like the table on /mem-trackers.
Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
---
M src/kudu/server/generic_service.cc
M src/kudu/server/generic_service.h
M src/kudu/server/server_base.proto
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_diagnose.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/tools/tool_action_tserver.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
A src/kudu/util/mem_tracker.proto
15 files changed, 422 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/11975/2
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.cc
File src/kudu/util/mem_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/11975/1/src/kudu/util/mem_tracker.cc@198
PS1, Line 198: to_process.top()
nit: maybe, might save a bit on copying if using std::move(to_process.top()) here?
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Nov 2018 19:00:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_common.h
File src/kudu/tools/tool_action_common.h:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_common.h@136
PS3, Line 136: Status DumpMemTrackers(const std::string& address, uint16_t default_port);
Doc.
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_master.cc@169
PS3, Line 169: .AddOptionalParameter("format")
Did you need to override the description for this? And in tool_action_tserver.cc?
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_tserver.cc
File src/kudu/tools/tool_action_tserver.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/tools/tool_action_tserver.cc@166
PS3, Line 166: tablet server
Nit: Kudu Tablet Server
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/util/mem_tracker-test.cc
File src/kudu/util/mem_tracker-test.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/util/mem_tracker-test.cc@192
PS3, Line 192: gc000->Consume(40);
: gc000->Release(20);
You can use ScopedTrackedConsumption here to avoid the SCOPED_CLEANUP below.
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Nov 2018 00:02:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/util/mem_tracker-test.cc
File src/kudu/util/mem_tracker-test.cc:
http://gerrit.cloudera.org:8080/#/c/11975/3/src/kudu/util/mem_tracker-test.cc@192
PS3, Line 192: gc000->Consume(40);
: gc000->Release(20);
> You can use ScopedTrackedConsumption here to avoid the SCOPED_CLEANUP below
On second thought, I think we want to promote usage of SCOPED_CLEANUP over bespoke cleanup objects.
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Nov 2018 00:03:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] Add tool to dump memtrackers
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11975 )
Change subject: [tools] Add tool to dump memtrackers
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e54f809bb6434b8d8e8c95771fe089c9da122d0
Gerrit-Change-Number: 11975
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 22:12:27 +0000
Gerrit-HasComments: No