You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/11/05 04:07:26 UTC
[kudu-CR] WIP: [metrics] Add metric severity level
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14630
Change subject: WIP: [metrics] Add metric severity level
......................................................................
WIP: [metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster the tablet
level metrics eventually become overwhelming for users and metric
collection tools. For example a server with 3777 live tablet replicas has
json metrics over 58 MiB.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
Additionally a configuration flag, `metrics_default_level`, was added with
a default value of `info` to drastically reduce the metrics shown by default.
This patch takes a quick first pass at setting metric levels, overtime
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
47 files changed, 586 insertions(+), 195 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/1
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [metrics] Add metric severity level
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG@34
PS4, Line 34: Signed-off-by: Grant Henke <gr...@apache.org>
This is unusual, I think
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 03:04:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#6).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster the tablet
level metrics eventually become overwhelming for users and metric
collection tools. For example a server with 3777 live tablet replicas has
json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
Additionally a configuration flag, `metrics_default_level`, was added with
a default value of `debug` to include all metrics by default but allow users
to configure the default to drastically reduce the metrics ouput.
This patch takes a quick first pass at setting metric levels, overtime
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 600 insertions(+), 201 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/6
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important. Additionally,
as the tablet count grows within the cluster, the tablet level metrics may
overwhelm users and some metric collection tools.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
This patch takes a quick first pass at setting metric levels, over time
the level of metrics can be adjusted based on their demand.
Some quick testing on a tablet server with 3777 live tablet replicas shows
a drastic reduction in size:
- Debug: 58.2 MB
- Info: 29.7 MB
- Warn: 3.3 MB
With `compact=true`:
- Debug: 26.2 MB
- Info: 13.6 MB
- Warn: 1.8 MB
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Reviewed-on: http://gerrit.cloudera.org:8080/14630
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 565 insertions(+), 208 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 14
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@30
PS6, Line 30: METRIC_DEFINE_counter(tablet, rows_inserted, "Rows Inserted",
: kudu::MetricUnit::kRows,
: "Number of rows inserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_upserted, "Rows Upserted",
: kudu::MetricUnit::kRows,
: "Number of rows upserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_updated, "Rows Updated",
: kudu::MetricUnit::kRows,
: "Number of row update operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_deleted, "Rows Deleted",
: kudu::MetricUnit::kRows,
: "Number of row delete operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
> Think that's good enough to check with level=debug then? Tablet metrics are
I do think these particular tablet metrics are worth it (as opposed to many others).
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 20:20:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/master/sentry_client_metrics.cc
File src/kudu/master/sentry_client_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/master/sentry_client_metrics.cc@29
PS3, Line 29: kudu::MetricLevel::kWarn);
> shouldn't this and the one below be warn or at least info?
Done
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/server/glog_metrics.cc
File src/kudu/server/glog_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/server/glog_metrics.cc@28
PS3, Line 28: kudu::MetricLevel::kDebug);
> do we need this to be info?
Done
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/tablet/tablet_metrics.cc@307
PS3, Line 307: kudu::MetricLevel::kInfo);
> should this be kInfo?
Done
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics-test.cc@961
PS3, Line 961: ASSERT_EQ(kEntityCount + 1, d.Size());
> wouldn't it be cleaner to increase kEntityCount by one and add +1 only in t
Since kEntityCount is defined for the whole class I didn't change it.
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics.cc@295
PS3, Line 295: return MetricLevel::kDebug;
> this and the WARNING message above should also be changed to kDebug until w
Done
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 05 Nov 2019 20:32:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: [metrics] Add metric severity level
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 12:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/hybrid_clock.cc@91
PS12, Line 91: kudu::MetricLevel::kInfo);
kDebug?
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/logical_clock.cc
File src/kudu/clock/logical_clock.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/logical_clock.cc@40
PS12, Line 40: kudu::MetricLevel::kInfo);
kDebug?
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/master/sentry_client_metrics.cc
File src/kudu/master/sentry_client_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/master/sentry_client_metrics.cc@41
PS12, Line 41: kudu::MetricLevel::kDebug);
kWarn?
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@63
PS12, Line 63: kudu::MetricLevel::kDebug);
Change to kInfo? Because input like rows_inserted/rows_upserted are kInfo level, output of scan should be kInfo too.
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@173
PS12, Line 173: kudu::MetricLevel::kDebug,
Change to kInfo? Because more delta file lookups may introduce more scan latency.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 12
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 13 Nov 2019 03:27:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG@26
PS4, Line 26: Additionally a configuration flag, `metrics_default_level`, was added with
: a default value of `debug` to include all metrics by default but allow users
: to configure the default to drastically reduce the metrics ouput.
> Instead of adding a new gflag to the kudu server to determine the metrics t
You can filter from the `/metrics` Web URL by adding `level=?`. This configuration is just the default when no level is specified.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 02:44:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#5).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster the tablet
level metrics eventually become overwhelming for users and metric
collection tools. For example a server with 3777 live tablet replicas has
json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
Additionally a configuration flag, `metrics_default_level`, was added with
a default value of `debug` to include all metrics by default but allow users
to configure the default to drastically reduce the metrics ouput.
This patch takes a quick first pass at setting metric levels, overtime
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 598 insertions(+), 201 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/5
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#12).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important. Additionally,
as the tablet count grows within the cluster, the tablet level metrics may
overwhelm users and some metric collection tools.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
This patch takes a quick first pass at setting metric levels, over time
the level of metrics can be adjusted based on their demand.
Some quick testing on a tablet server with 3777 live tablet replicas shows
a drastic reduction in size:
- Debug: 58.2 MB
- Info: 29.7 MB
- Warn: 3.3 MB
With `compact=true`:
- Debug: 26.2 MB
- Info: 13.6 MB
- Warn: 1.8 MB
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 565 insertions(+), 208 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/12
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 12
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@30
PS6, Line 30: METRIC_DEFINE_counter(tablet, rows_inserted, "Rows Inserted",
: kudu::MetricUnit::kRows,
: "Number of rows inserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_upserted, "Rows Upserted",
: kudu::MetricUnit::kRows,
: "Number of rows upserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_updated, "Rows Updated",
: kudu::MetricUnit::kRows,
: "Number of row update operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_deleted, "Rows Deleted",
: kudu::MetricUnit::kRows,
: "Number of row delete operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
> Agreed that their utility is lessened because they reset on restart, but th
Think that's good enough to check with level=debug then? Tablet metrics are particularly bloaty given they are multiplied by the number of tablets. So I want to be more selective with them.
If you say "yeah I think they are worth it", I will add them. Just wanted to double check.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 02:37:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 13
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 14 Nov 2019 18:54:53 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/master/sentry_client_metrics.cc
File src/kudu/master/sentry_client_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/master/sentry_client_metrics.cc@29
PS3, Line 29: kudu::MetricLevel::kDebug);
shouldn't this and the one below be warn or at least info?
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/server/glog_metrics.cc
File src/kudu/server/glog_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/server/glog_metrics.cc@28
PS3, Line 28: kudu::MetricLevel::kInfo);
do we need this to be info?
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/tablet/tablet_metrics.cc@307
PS3, Line 307: kudu::MetricLevel::kDebug);
should this be kInfo?
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics-test.cc@961
PS3, Line 961: ASSERT_EQ(kEntityCount + 1, d.Size());
wouldn't it be cleaner to increase kEntityCount by one and add +1 only in the debug case?
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/metrics.cc@295
PS3, Line 295: return MetricLevel::kInfo;
this and the WARNING message above should also be changed to kDebug until we increase the default level.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 05 Nov 2019 18:27:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@30
PS6, Line 30: METRIC_DEFINE_counter(tablet, rows_inserted, "Rows Inserted",
: kudu::MetricUnit::kRows,
: "Number of rows inserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_upserted, "Rows Upserted",
: kudu::MetricUnit::kRows,
: "Number of rows upserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_updated, "Rows Updated",
: kudu::MetricUnit::kRows,
: "Number of row update operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_deleted, "Rows Deleted",
: kudu::MetricUnit::kRows,
: "Number of row delete operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
> Why are these especially useful? Given they are cumulative since service st
Agreed that their utility is lessened because they reset on restart, but they're still useful as very quick "I just tested that I could create a table; did _some_ metric change as a result of that?"
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc@334
PS7, Line 334: if (static_cast<int>(metric->first->level()) < lowest) {
You could avoid all the casting if you defined operator< for the enum class. Alternatively, you could define a function that converts the enum class into an int using a simple switch.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 23:05:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@30
PS6, Line 30: METRIC_DEFINE_counter(tablet, rows_inserted, "Rows Inserted",
: kudu::MetricUnit::kRows,
: "Number of rows inserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_upserted, "Rows Upserted",
: kudu::MetricUnit::kRows,
: "Number of rows upserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_updated, "Rows Updated",
: kudu::MetricUnit::kRows,
: "Number of row update operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_deleted, "Rows Deleted",
: kudu::MetricUnit::kRows,
: "Number of row delete operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
> I do think these particular tablet metrics are worth it (as opposed to many
Done
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 05:10:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 10: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 10
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 15:14:15 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 12:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@50
PS12, Line 50: kudu::MetricLevel::kDebug);
How about change to kInfo or even kWarn level?
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@55
PS12, Line 55: "existed.",
align, and change to kInfo level? Because update will append items to redo log which may introduce more latency.
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/util/metrics.cc@335
PS12, Line 335: int lowest = MetricLevelNumeric(MetricLevel::kDebug); // Default to the lowest level.
: if (!filters.entity_level.empty()) {
: if (MatchName(MetricLevelName(MetricLevel::kDebug), filters.entity_level)) {
: lowest = MetricLevelNumeric(MetricLevel::kDebug);
: } else if (MatchName(MetricLevelName(MetricLevel::kInfo), filters.entity_level)) {
: lowest = MetricLevelNumeric(MetricLevel::kInfo);
: } else if (MatchName(MetricLevelName(MetricLevel::kWarn), filters.entity_level)) {
: lowest = MetricLevelNumeric(MetricLevel::kWarn);
: }
: }
How about wrap these code to a function like 'int GetMetricLevelNumeric(const string& level)' ?
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 12
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 13 Nov 2019 02:57:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 12
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 21:21:02 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc@334
PS7, Line 334: if (static_cast<int>(metric->first->level()) < lowest) {
> Done
And another way is to use simple enum (not enum class), achieving the same with much less code.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 19:25:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#3).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster the tablet
level metrics eventually become overwhelming for users and metric
collection tools. For example a server with 3777 live tablet replicas has
json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
Additionally a configuration flag, `metrics_default_level`, was added with
a default value of `debug` to include all metrics by default but allow users
to configure the default to drastically reduce the metrics ouput.
This patch takes a quick first pass at setting metric levels, overtime
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 599 insertions(+), 201 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/3
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] Add metric severity level
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 12: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 12
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 19:29:08 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 4:
(1 comment)
Pretty neat! LGTM overall
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc@292
PS4, Line 292:
: LOG(WARNING) << "Unable to recognize the default metric level '"
: << FLAGS_metrics_default_level << "', using debug as default.";
: return MetricLevel::kDebug;
: }
:
This will probably be ignored and buried in logs. How could we add a validator instead?
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 03:03:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#4).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster the tablet
level metrics eventually become overwhelming for users and metric
collection tools. For example a server with 3777 live tablet replicas has
json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
Additionally a configuration flag, `metrics_default_level`, was added with
a default value of `debug` to include all metrics by default but allow users
to configure the default to drastically reduce the metrics ouput.
This patch takes a quick first pass at setting metric levels, overtime
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Signed-off-by: Grant Henke <gr...@apache.org>
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 598 insertions(+), 201 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/4
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc@334
PS7, Line 334: if (static_cast<int>(metric->first->level()) < lowest) {
> You could avoid all the casting if you defined operator< for the enum class
Done
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 02:53:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 10: Code-Review+2
You may want to check with Yingchun before merging in case he wants to review.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 10
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 06:02:20 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/1//COMMIT_MSG@26
PS1, Line 26: Additionally a configuration flag, `metrics_default_level`, was added with
: a default value of `info` to drastically reduce the metrics shown by default.
> Haven't looked at the patch yet, but wanted to say that while configurable
I am happy to decouple changing the default into a separate change.
Ideally, it would be done before the next release to help alleviate pain we have seen from massive metrics output, but I will collect feedback and good metrics on the size impact before making that change.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 05 Nov 2019 17:29:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#8).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster, the tablet level
metrics may overwhelm users and some metric collection tools. For example
a server with 3777 live tablet replicas has json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
This patch takes a quick first pass at setting metric levels, over time
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 566 insertions(+), 208 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/8
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#13).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important. Additionally,
as the tablet count grows within the cluster, the tablet level metrics may
overwhelm users and some metric collection tools.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
This patch takes a quick first pass at setting metric levels, over time
the level of metrics can be adjusted based on their demand.
Some quick testing on a tablet server with 3777 live tablet replicas shows
a drastic reduction in size:
- Debug: 58.2 MB
- Info: 29.7 MB
- Warn: 3.3 MB
With `compact=true`:
- Debug: 26.2 MB
- Info: 13.6 MB
- Warn: 1.8 MB
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 565 insertions(+), 208 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/13
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 13
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG@34
PS4, Line 34: Signed-off-by: Grant Henke <gr...@apache.org>
> This is unusual, I think
Weird, not sure how that got there.
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc@292
PS4, Line 292:
: LOG(WARNING) << "Unable to recognize the default metric level '"
: << FLAGS_metrics_default_level << "', using debug as default.";
: return MetricLevel::kDebug;
: }
:
> This will probably be ignored and buried in logs. How could we add a valida
I didn't think failing the server at startup was worth it just for a metric filter configuration.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 03:20:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 12:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/hybrid_clock.cc@91
PS12, Line 91: kudu::MetricLevel::kInfo);
> kDebug?
A previous reviewer suggested I bump to kInfo so I will leave it for now.
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/logical_clock.cc
File src/kudu/clock/logical_clock.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/clock/logical_clock.cc@40
PS12, Line 40: kudu::MetricLevel::kInfo);
> kDebug?
A previous reviewer suggested I bump to kInfo so I will leave it for now.
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/master/sentry_client_metrics.cc
File src/kudu/master/sentry_client_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/master/sentry_client_metrics.cc@41
PS12, Line 41: kudu::MetricLevel::kDebug);
> kWarn?
I think this would result in failed tasks, so it's covered by another metric.
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@50
PS12, Line 50: kudu::MetricLevel::kDebug);
> How about change to kInfo or even kWarn level?
Given these are errors exposed to the client, I didn't think they were critical metrics except in debugging situations.
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@55
PS12, Line 55: "existed.",
> align, and change to kInfo level? Because update will append items to redo
Done
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@63
PS12, Line 63: kudu::MetricLevel::kDebug);
> Change to kInfo? Because input like rows_inserted/rows_upserted are kInfo l
Given there are often many concurrent scans and this is cumulative I think these are likely to be less commonly used unless in a specific debugging scenario.
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/tablet/tablet_metrics.cc@173
PS12, Line 173: kudu::MetricLevel::kDebug,
> Change to kInfo? Because more delta file lookups may introduce more scan la
Done
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/12/src/kudu/util/metrics.cc@335
PS12, Line 335: int lowest = MetricLevelNumeric(MetricLevel::kDebug); // Default to the lowest level.
: if (!filters.entity_level.empty()) {
: if (MatchName(MetricLevelName(MetricLevel::kDebug), filters.entity_level)) {
: lowest = MetricLevelNumeric(MetricLevel::kDebug);
: } else if (MatchName(MetricLevelName(MetricLevel::kInfo), filters.entity_level)) {
: lowest = MetricLevelNumeric(MetricLevel::kInfo);
: } else if (MatchName(MetricLevelName(MetricLevel::kWarn), filters.entity_level)) {
: lowest = MetricLevelNumeric(MetricLevel::kWarn);
: }
: }
> How about wrap these code to a function like 'int GetMetricLevelNumeric(con
I think this is okay since it's the only place it's used.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 12
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 14 Nov 2019 16:48:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 3:
> Patch Set 3:
>
> (5 comments)
LGTM overall, left a few notes above
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 05 Nov 2019 18:27:55 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 05 Nov 2019 21:46:17 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 5:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/rpc/reactor.cc
File src/kudu/rpc/reactor.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/rpc/reactor.cc@106
PS5, Line 106: kWarn
kInfo?
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/rpc/reactor.cc@116
PS5, Line 116: kWarn
kInfo?
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/tserver/ts_tablet_manager.cc@176
PS5, Line 176: kWarn
Out of all these tablet-related metrics, only this looks of warning level to me, others are seem to be more in the info realm.
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/block_cache_metrics.cc
File src/kudu/util/block_cache_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/block_cache_metrics.cc@25
PS5, Line 25: kInfo
I think all these 'kInfo' should be rather 'kDebug'.
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/block_cache_metrics.cc@58
PS5, Line 58: kWarn
I think this kWarn is rather kInfo. The cache has a limited capacity, and the usage isn't not going to get over that limit anyways.
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/file_cache_metrics.cc
File src/kudu/util/file_cache_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/file_cache_metrics.cc@25
PS5, Line 25: kInfo
I think all these metrics of kInfo level are rather kDebug.
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/file_cache_metrics.cc@59
PS5, Line 59: kWarn
It's rather kInfo: the cache usage is not going to get over cache's capacity anyways.
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics-test.cc@871
PS5, Line 871: ASSERT_EQ(kNum + kEntityCount + 2, d.Size());
Is this going to break when the default level changes? Maybe, set the FLAGS_metrics_default_level flag explicitly in this test since the default filter value is covered by explicit test at line 995?
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics-test.cc@979
PS5, Line 979: opts.filters.entity_levels = { entity_level };
Could you add the test to exercise the case of multiple entity levels in filters? At least it would help to get better understanding of the semantics of the multitude of those.
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics.h
File src/kudu/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics.h@434
PS5, Line 434: static const int Numeric(Level level);
Why is it needed? In C++, there is implicit conversion for such enums (since it's not a class enum). If exact values are desired, you could have something like
enum Level {
kDebug = 0,
...
};
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc@292
PS4, Line 292:
: LOG(WARNING) << "Unable to recognize the default metric level '"
: << FLAGS_metrics_default_level << "', using debug as default.";
: return MetricLevel::kDebug;
: }
:
> I didn't think failing the server at startup was worth it just for a metric
+1 for adding a validator. Regardless of what the flag controls, detecting misconfiguration early helps saving some on troubleshooting.
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics.cc@42
PS5, Line 42: metrics_default_level
Maybe, add a validator for this flag?
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/thread.cc@88
PS3, Line 88: kWarn
kInfo ? Why number of threads is so important compared, say, to System CPU Time?
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/thread.cc@88
PS5, Line 88: kWarn
Why not kInfo?
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/ttl_cache_test_metrics.cc
File src/kudu/util/ttl_cache_test_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/ttl_cache_test_metrics.cc@26
PS5, Line 26: kInfo
I think this should be in line with the severity of metrics in cfile and block caches: all except for usage is kDebug, and usage is kInfo.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 23:01:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 12: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc@334
PS7, Line 334: // Includes all levels above the lowest matched level.
> I already went to and from that style of enum based on feedback. Feel free
SGTM
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 12
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 13 Nov 2019 02:20:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: WIP: [metrics] Add metric severity level
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/1//COMMIT_MSG@26
PS1, Line 26: Additionally a configuration flag, `metrics_default_level`, was added with
: a default value of `info` to drastically reduce the metrics shown by default.
Haven't looked at the patch yet, but wanted to say that while configurable severity for metrics is useful, we should collect feedback from users to see whether there's consensus for reducing the metric volume by default.
One option could be to introduce the severity level in one release, then change the default in the next after soliciting feedback. Or you could try to collect that feedback now.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 05 Nov 2019 08:26:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 6:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/14630/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/6//COMMIT_MSG@13
PS6, Line 13: Additionally, as the tablet count grows within the cluster the tablet
: level metrics eventually become overwhelming for users and metric
: collection tools
I'd rephrase this a bit: "Additionally, as the tablet count grows within the cluster, the tablet level metrics may overwhelm users and some metric collection tools."
http://gerrit.cloudera.org:8080/#/c/14630/6//COMMIT_MSG@30
PS6, Line 30: overtime
over time
http://gerrit.cloudera.org:8080/#/c/14630/6/docs/administration.adoc
File docs/administration.adoc:
http://gerrit.cloudera.org:8080/#/c/14630/6/docs/administration.adoc@111
PS6, Line 111: - `/metrics?level=info` - limits the returned metrics based on their severity level. The valid
Should probably doc the default level here too.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_metrics.cc
File src/kudu/consensus/log_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_metrics.cc@30
PS6, Line 30: kudu::MetricLevel::kDebug,
Likewise.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_metrics.cc@36
PS6, Line 36: kudu::MetricLevel::kDebug,
This one is more useful, kInfo?
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_reader.cc
File src/kudu/consensus/log_reader.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_reader.cc@55
PS6, Line 55: kudu::MetricLevel::kDebug,
kInfo
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/master/sentry_client_metrics.cc
File src/kudu/master/sentry_client_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/master/sentry_client_metrics.cc@46
PS6, Line 46: kudu::MetricLevel::kDebug,
kInfo on this one too.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@30
PS6, Line 30: METRIC_DEFINE_counter(tablet, rows_inserted, "Rows Inserted",
: kudu::MetricUnit::kRows,
: "Number of rows inserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_upserted, "Rows Upserted",
: kudu::MetricUnit::kRows,
: "Number of rows upserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_updated, "Rows Updated",
: kudu::MetricUnit::kRows,
: "Number of row update operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_deleted, "Rows Deleted",
: kudu::MetricUnit::kRows,
: "Number of row delete operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
I think these should be kInfo.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@247
PS6, Line 247: METRIC_DEFINE_histogram(tablet, flush_dms_duration,
These histograms are super useful; they should be kInfo.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@307
PS6, Line 307: kudu::MetricLevel::kInfo);
kWarn, this could indicate too much memory pressure.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica.cc@64
PS6, Line 64: METRIC_DEFINE_histogram(tablet, op_prepare_queue_length, "Operation Prepare Queue Length",
: kudu::MetricUnit::kTasks,
: "Number of operations waiting to be prepared within this tablet. "
: "High queue lengths indicate that the server is unable to process "
: "operations as fast as they are being written to the WAL.",
: kudu::MetricLevel::kDebug,
: 10000, 2);
:
: METRIC_DEFINE_histogram(tablet, op_prepare_queue_time, "Operation Prepare Queue Time",
: kudu::MetricUnit::kMicroseconds,
: "Time that operations spent waiting in the prepare queue before being "
: "processed. High queue times indicate that the server is unable to "
: "process operations as fast as they are being written to the WAL.",
: kudu::MetricLevel::kDebug,
: 10000000, 2);
:
: METRIC_DEFINE_histogram(tablet, op_prepare_run_time, "Operation Prepare Run Time",
: kudu::MetricUnit::kMicroseconds,
: "Time that operations spent being prepared in the tablet. "
: "High values may indicate that the server is under-provisioned or "
: "that operations are experiencing high contention with one another for "
: "locks.",
: kudu::MetricLevel::kDebug,
: 10000000, 2);
kInfo, these are often useful in diagnosing lock contention (i.e. several "hot" keys).
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica_mm_ops.cc
File src/kudu/tablet/tablet_replica_mm_ops.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica_mm_ops.cc@84
PS6, Line 84: METRIC_DEFINE_histogram(tablet, log_gc_duration,
Generally speaking I think all histograms associated with MM ops should be kInfo, and this is no exception.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h
File src/kudu/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h@420
PS6, Line 420: generally
Nit: second use of 'generally', drop
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h@427
PS6, Line 427: struct MetricLevel {
Why wrap it in a struct? You could define Name as a free function and just have the enum.
BTW for new code we prefer enum classes to enums. Then you won't need the struct inasfar as scoping is concerned (because you'd refer to enum values as Level::kDebug).
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h@452
PS6, Line 452: // entity level.
: std::vector<std::string> entity_levels;
Why a vector and not just a Level? Doesn't one level imply all the levels before it?
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc@292
PS4, Line 292:
: // Filter the 'type'.
: if (!filters.entity_types.empty() && !MatchNameInList(prototype_->name(), filters.entity_types)) {
: return Status::NotFound("entity is filtered by entity type");
: }
:
> Given this configuration is not critical to normal operation and the log le
I also agree with adding a validator. There are many gflags whose values aren't critical to normal operation, and yet we validate their values at startup so that if the user fat-fingers them, they'll know about it sooner rather than later. If a node or cluster is already being restarted, why would a crash from an invalid config matter? There was already an "outage" for the restart itself, right?
That said, after reviewing the change in its entirety, I think we don't want a gflag for this at all (see my other comment).
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.cc@41
PS6, Line 41:
: DEFINE_string(metrics_default_level, "debug",
: "The default severity level to use when filtering the metrics. "
: "Valid choices are 'debug', 'info', and 'warn'. "
: "This value can be overridden by passing the level parameter to the "
: "'/metrics' endpoint.");
: TAG_FLAG(metrics_default_level, runtime);
: TAG_FLAG(metrics_default_level, evolving);
On second thought, I don't think we want a gflag for this at all. Level-based filtering should be indicated by a client when fetching metrics (i.e. via ?level=whatever). Different clients/users are going to have different needs, and thus far we've managed to address all of those disparate needs via query parameters.
For example, the metrics log that every Kudu server automatically collects should always use kDebug because it already omits all metrics whose values haven't changed since the previous collection.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 17:49:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#7).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster, the tablet level
metrics may overwhelm users and some metric collection tools. For example
a server with 3777 live tablet replicas has json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
This patch takes a quick first pass at setting metric levels, over time
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 553 insertions(+), 208 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/7
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 03:49:46 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 6:
Yingchun has done a lot of work on the metrics subsystem; he might be interested in looking at this too.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 17:50:09 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 15:14:00 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#9).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster, the tablet level
metrics may overwhelm users and some metric collection tools. For example
a server with 3777 live tablet replicas has json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
This patch takes a quick first pass at setting metric levels, over time
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 565 insertions(+), 208 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/9
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] WIP: [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#2).
Change subject: WIP: [metrics] Add metric severity level
......................................................................
WIP: [metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster the tablet
level metrics eventually become overwhelming for users and metric
collection tools. For example a server with 3777 live tablet replicas has
json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
Additionally a configuration flag, `metrics_default_level`, was added with
a default value of `info` to drastically reduce the metrics shown by default.
This patch takes a quick first pass at setting metric levels, overtime
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/kserver/kserver.cc
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/test_util.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
51 files changed, 606 insertions(+), 200 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/2
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/7/src/kudu/util/metrics.cc@334
PS7, Line 334: if (static_cast<int>(metric->first->level()) < lowest) {
> And another way is to use simple enum (not enum class), achieving the same
I already went to and from that style of enum based on feedback. Feel free to make changes in a follow on commit to match your preference.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 12 Nov 2019 19:36:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 06:33:53 +0000
Gerrit-HasComments: No
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: [metrics] Add metric severity level
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "helifu (Code Review)" <ge...@cloudera.org>.
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG@26
PS4, Line 26: Additionally a configuration flag, `metrics_default_level`, was added with
: a default value of `debug` to include all metrics by default but allow users
: to configure the default to drastically reduce the metrics ouput.
Instead of adding a new gflag to the kudu server to determine the metrics to return, how about adding a new filter from the Web URL? I mean we can config the severity level for the metrics, but it is better to let the client decide which level of metrics they need.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 02:38:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Attila Bukor, Kudu Jenkins, Andrew Wong, helifu, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14630
to look at the new patch set (#10).
Change subject: [metrics] Add metric severity level
......................................................................
[metrics] Add metric severity level
Kudu has somewhere around 200 metrics defined for the server and
tablet entities. The result is a lot of useful information. However, it is also
difficult to view and understand which metrics are important.
Additionally, as the tablet count grows within the cluster, the tablet level
metrics may overwhelm users and some metric collection tools. For example
a server with 3777 live tablet replicas has json metrics over 58 MiB in size.
Kudu already has the ability to filter metrics by type, attribute, name, etc.
This patch adds the additional capability to filter by metric “severity level”.
In order to do this, this patch introduces three metric severity levels:
debug, info, and warn. These levels are defined similar to logging
severity levels. See the `MetricLevel` documentation in `metrics.h` for
a more detailed definition of each level.
This patch takes a quick first pass at setting metric levels, over time
the level of metrics can be adjusted based on their demand.
Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
---
M docs/administration.adoc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/logical_clock.cc
M src/kudu/clock/logical_clock.h
M src/kudu/codegen/compilation_manager.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_metrics.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/location_cache.cc
M src/kudu/master/sentry_client_metrics.cc
M src/kudu/master/sentry_privileges_cache_metrics.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/protoc-gen-krpc.cc
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/service_pool.cc
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/glog_metrics.cc
M src/kudu/server/tcmalloc_metrics.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica_mm_ops.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tserver/scanner_metrics.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/block_cache_metrics.cc
M src/kudu/util/file_cache_metrics.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/spinlock_profiling.cc
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/ttl_cache_test_metrics.cc
50 files changed, 565 insertions(+), 208 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/14630/10
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 10
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 7:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/14630/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14630/6//COMMIT_MSG@13
PS6, Line 13: Additionally, as the tablet count grows within the cluster, the tablet level
: metrics may overwhelm users and some metric collection tools. For example
: a server with 37
> I'd rephrase this a bit: "Additionally, as the tablet count grows within th
Done
http://gerrit.cloudera.org:8080/#/c/14630/6//COMMIT_MSG@30
PS6, Line 30:
> over time
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/docs/administration.adoc
File docs/administration.adoc:
http://gerrit.cloudera.org:8080/#/c/14630/6/docs/administration.adoc@111
PS6, Line 111: - `/metrics?level=info` - limits the returned metrics based on their severity level.
> Should probably doc the default level here too.
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_metrics.cc
File src/kudu/consensus/log_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_metrics.cc@30
PS6, Line 30: kudu::MetricLevel::kInfo,
> Likewise.
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_metrics.cc@36
PS6, Line 36: kudu::MetricLevel::kInfo,
> This one is more useful, kInfo?
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_reader.cc
File src/kudu/consensus/log_reader.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/consensus/log_reader.cc@55
PS6, Line 55: kudu::MetricLevel::kInfo,
> kInfo
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/master/sentry_client_metrics.cc
File src/kudu/master/sentry_client_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/master/sentry_client_metrics.cc@46
PS6, Line 46: kudu::MetricLevel::kInfo,
> kInfo on this one too.
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@30
PS6, Line 30: METRIC_DEFINE_counter(tablet, rows_inserted, "Rows Inserted",
: kudu::MetricUnit::kRows,
: "Number of rows inserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_upserted, "Rows Upserted",
: kudu::MetricUnit::kRows,
: "Number of rows upserted into this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_updated, "Rows Updated",
: kudu::MetricUnit::kRows,
: "Number of row update operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
: METRIC_DEFINE_counter(tablet, rows_deleted, "Rows Deleted",
: kudu::MetricUnit::kRows,
: "Number of row delete operations performed on this tablet since service start",
: kudu::MetricLevel::kDebug);
> I think these should be kInfo.
Why are these especially useful? Given they are cumulative since service start I find them harder to use and interpret.
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@247
PS6, Line 247: METRIC_DEFINE_histogram(tablet, flush_dms_duration,
> These histograms are super useful; they should be kInfo.
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_metrics.cc@307
PS6, Line 307: kudu::MetricLevel::kWarn);
> kWarn, this could indicate too much memory pressure.
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica.cc@64
PS6, Line 64: METRIC_DEFINE_histogram(tablet, op_prepare_queue_length, "Operation Prepare Queue Length",
: kudu::MetricUnit::kTasks,
: "Number of operations waiting to be prepared within this tablet. "
: "High queue lengths indicate that the server is unable to process "
: "operations as fast as they are being written to the WAL.",
: kudu::MetricLevel::kInfo,
: 10000, 2);
:
: METRIC_DEFINE_histogram(tablet, op_prepare_queue_time, "Operation Prepare Queue Time",
: kudu::MetricUnit::kMicroseconds,
: "Time that operations spent waiting in the prepare queue before being "
: "processed. High queue times indicate that the server is unable to "
: "process operations as fast as they are being written to the WAL.",
: kudu::MetricLevel::kInfo,
: 10000000, 2);
:
: METRIC_DEFINE_histogram(tablet, op_prepare_run_time, "Operation Prepare Run Time",
: kudu::MetricUnit::kMicroseconds,
: "Time that operations spent being prepared in the tablet. "
: "High values may indicate that the server is under-provisioned or "
: "that operations are experiencing high contention with one another for "
: "locks.",
: kudu::MetricLevel::kInfo,
: 10000000, 2);
> kInfo, these are often useful in diagnosing lock contention (i.e. several "
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica_mm_ops.cc
File src/kudu/tablet/tablet_replica_mm_ops.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/tablet/tablet_replica_mm_ops.cc@84
PS6, Line 84: METRIC_DEFINE_histogram(tablet, log_gc_duration,
> Generally speaking I think all histograms associated with MM ops should be
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h
File src/kudu/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h@420
PS6, Line 420: always wa
> Nit: second use of 'generally', drop
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h@427
PS6, Line 427: enum class MetricLevel {
> Why wrap it in a struct? You could define Name as a free function and just
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.h@452
PS6, Line 452:
: struct MergeAttributes {
> Why a vector and not just a Level? Doesn't one level imply all the levels b
Done
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/6/src/kudu/util/metrics.cc@41
PS6, Line 41:
: // Process/server-wide metrics should go into the 'server' entity.
: // More complex applications will define other entities.
: METRIC_DEFINE_entity(server);
:
: namespace kudu {
:
: using std::string;
> On second thought, I don't think we want a gflag for this at all. Level-bas
The idea here was that if we know certain metrics aren't commonly used and introduce significant load at scale (either through logging or URL) we could omit them by default but allow users to opt-in to seeing them.
I can remove the flag for now. We can add it back if we find it useful.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 22:44:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Add metric severity level
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 )
Change subject: [metrics] Add metric severity level
......................................................................
Patch Set 5:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/rpc/reactor.cc
File src/kudu/rpc/reactor.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/rpc/reactor.cc@106
PS5, Line 106: kWarn
> kInfo?
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/rpc/reactor.cc@116
PS5, Line 116: kWarn
> kInfo?
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/tserver/ts_tablet_manager.cc@176
PS5, Line 176: kWarn
> Out of all these tablet-related metrics, only this looks of warning level t
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/block_cache_metrics.cc
File src/kudu/util/block_cache_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/block_cache_metrics.cc@25
PS5, Line 25: kInfo
> I think all these 'kInfo' should be rather 'kDebug'.
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/block_cache_metrics.cc@58
PS5, Line 58: kWarn
> I think this kWarn is rather kInfo. The cache has a limited capacity, and
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/file_cache_metrics.cc
File src/kudu/util/file_cache_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/file_cache_metrics.cc@25
PS5, Line 25: kInfo
> I think all these metrics of kInfo level are rather kDebug.
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/file_cache_metrics.cc@59
PS5, Line 59: kWarn
> It's rather kInfo: the cache usage is not going to get over cache's capacit
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics-test.cc
File src/kudu/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics-test.cc@871
PS5, Line 871: ASSERT_EQ(kNum + kEntityCount + 2, d.Size());
> Is this going to break when the default level changes? Maybe, set the FLAG
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics-test.cc@979
PS5, Line 979: opts.filters.entity_levels = { entity_level };
> Could you add the test to exercise the case of multiple entity levels in fi
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics.h
File src/kudu/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/metrics.h@434
PS5, Line 434: static const int Numeric(Level level);
> Why is it needed? In C++, there is implicit conversion for such enums (sin
Done
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/4/src/kudu/util/metrics.cc@292
PS4, Line 292:
: LOG(WARNING) << "Unable to recognize the default metric level '"
: << FLAGS_metrics_default_level << "', using debug as default.";
: return MetricLevel::kDebug;
: }
:
> +1 for adding a validator. Regardless of what the flag controls, detecting
Given this configuration is not critical to normal operation and the log level is runtime changeable (both the config and URL) I think it's a bit extreme to potentially impact availability and cause an outage on a restart for this config.
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:
http://gerrit.cloudera.org:8080/#/c/14630/3/src/kudu/util/thread.cc@88
PS3, Line 88: kWarn
> kInfo ? Why number of threads is so important compared, say, to System CPU
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/thread.cc@88
PS5, Line 88: kWarn
> Why not kInfo?
Done
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/ttl_cache_test_metrics.cc
File src/kudu/util/ttl_cache_test_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14630/5/src/kudu/util/ttl_cache_test_metrics.cc@26
PS5, Line 26: kInfo
> I think this should be in line with the severity of metrics in cfile and bl
I think these are metrics only used in testing so no need to lower the level and make test setup more challenging.
--
To view, visit http://gerrit.cloudera.org:8080/14630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715
Gerrit-Change-Number: 14630
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 17:08:19 +0000
Gerrit-HasComments: Yes