Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DocDB] server->root mem tracker metric has the same name as per-tablet mem tracker metric #18627

Closed
1 task done
yusong-yan opened this issue Aug 9, 2023 · 0 comments
Closed
1 task done
Assignees
Labels
2.18 Backport Required area/docdb YugabyteDB core features kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue

Comments

@yusong-yan
Copy link
Contributor

yusong-yan commented Aug 9, 2023

Jira Link: DB-7554

Description

We use mem_tracker as the metric name for server->root mem_tracker metric:

# HELP mem_tracker Memory consumed by server->root
# TYPE mem_tracker gauge
mem_tracker{exported_instance="dev-server-yyan:9000",metric_type="server",metric_id="yb.tabletserver"} 67584 1691593596294

However, for per-tablet mem_tracker metric, its metric name is also mem_tracker:

# HELP mem_tracker Memory consumed by tablet-b5e50725f5fe4c19a67c1002bd8037bd->Tablets->server->root
# TYPE mem_tracker gauge
mem_tracker{exported_instance="dev-server-yyan:9000",metric_type="table",table_type="YQL_TABLE_TYPE",namespace_name="system",table_name="metrics",table_id="94b954d5b73948c29ab022fe65bea772"} 67584 1691593596294

The correct name for per-tablet one should be mem_tracker_Tablets_tablet

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@yusong-yan yusong-yan added area/docdb YugabyteDB core features status/awaiting-triage Issue awaiting triage labels Aug 9, 2023
@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue labels Aug 9, 2023
@yusong-yan yusong-yan self-assigned this Aug 9, 2023
@yugabyte-ci yugabyte-ci removed the status/awaiting-triage Issue awaiting triage label Aug 15, 2023
yusong-yan added a commit that referenced this issue Sep 6, 2023
Summary:
The current way of creating mem_tracker metric names is to keep traversing parent’s tracker and grab its mem tracker id until a parent’s metric entity is different from the current tracker. And the issue here is:
```
# HELP mem_tracker Memory consumed by tablet-b5e50725f5fe4c19a67c1002bd8037bd->Tablets->server->root
# TYPE mem_tracker gauge
mem_tracker{exported_instance="dev-server-yyan:9000",metric_type="table",table_type="YQL_TABLE_TYPE",namespace_name="system",table_name="metrics",table_id="94b954d5b73948c29ab022fe65bea772"} 67584 1691593596294
```
Since the per tablet mem_tracker has a tablet metric entity, and its parent has a server entity, it wouldn't traverse the parent tracker and just uses “mem_tracker” as per the tablet mem_tracker metric name, which is same as server root tracker’s metric name:
```
# HELP mem_tracker Memory consumed by server->root
# TYPE mem_tracker gauge
mem_tracker{exported_instance="dev-server-yyan:9000",metric_type="server",metric_id="yb.tabletserver"} 67584 1691593596294
```
This was introduced by D5718

**Fix:**
It makes sense to just crawl to the root without considering the metric entity. For example, per tablet tracker’s new metric name will be:
“`mem_tracker_server_Tablets_tablet_<tablet_id>`”. However, since its metric attribute already contains tablet_id, a better name should be “`mem_tracker_server_Tablets_PerTablet`”. Plus, another benefit is, without tablet_id in the metric name, we can also aggregate this metric to table-level. Now, users now can customize a MemTracker’s metric name rather than use its original tracker name for metric.
Here are some examples of new metric names:

tablet-f5b060aaca0041e0a8a5f0cbfc28dead->Tablets->server->root
Old: `mem_tracker{}`
New: `mem_tracker_server_Tablets_PerTablet{}`

OperationsFromDisk->tablet-f5b060aaca0041e0a8a5f0cbfc28dead>Tablets->server->root
Old: `mem_tracker_OperationsFromDisk{}`
New: `mem_tracker_server_Tablets_PerTablet_OperationsFromDisk{}`

LogCache-f5b060aaca0041e0a8a5f0cbfc28dead->LogCache->server->root
Old: `mem_tracker_log_cache{}`
New: `mem_tracker_server_LogCache_PerLogCache{}`

/metrics from a tserver and master: [[ https://gist.github.com/yusong-yan/3ae57a5b55aabece3da4aaf586a0d9fe | github gist ]]

This diff also fixed the name issue that was introduced by D26876, where it modified the `server->root` MemTracker's metric name from `mem_tracker` to `root_server`.
Jira: DB-7554

Test Plan: Validated on dev server

Reviewers: asrivastava, rthallam, esheng

Reviewed By: esheng

Subscribers: yql, ybase, bogdan

Differential Revision: https://phorge.dev.yugabyte.com/D27947
yusong-yan added a commit that referenced this issue Nov 6, 2023
…name creation

Summary:
Original commit: 156c263 / D27947
The current way of creating mem_tracker metric names is to keep traversing parent’s tracker and grab its mem tracker id until a parent’s metric entity is different from the current tracker. And the issue here is:
```
mem_tracker{exported_instance="dev-server-yyan:9000",metric_type="table",table_type="YQL_TABLE_TYPE",namespace_name="system",table_name="metrics",table_id="94b954d5b73948c29ab022fe65bea772"} 67584 1691593596294
```
Since the per tablet mem_tracker has a tablet metric entity, and its parent has a server entity, it wouldn't traverse the parent tracker and just uses “mem_tracker” as per the tablet mem_tracker metric name, which is same as server root tracker’s metric name:
```
mem_tracker{exported_instance="dev-server-yyan:9000",metric_type="server",metric_id="yb.tabletserver"} 67584 1691593596294
```
This was introduced by D5718

**Fix:**
It makes sense to just crawl to the root without considering the metric entity. For example, per tablet tracker’s new metric name will be:
“`mem_tracker_server_Tablets_tablet_<tablet_id>`”. However, since its metric attribute already contains tablet_id, a better name should be “`mem_tracker_server_Tablets_PerTablet`”. Plus, another benefit is, without tablet_id in the metric name, we can also aggregate this metric to table-level. Now, users now can customize a MemTracker’s metric name rather than use its original tracker name for metric.
Here are some examples of new metric names:

tablet-f5b060aaca0041e0a8a5f0cbfc28dead->Tablets->server->root
Old: `mem_tracker{}`
New: `mem_tracker_server_Tablets_PerTablet{}`

OperationsFromDisk->tablet-f5b060aaca0041e0a8a5f0cbfc28dead>Tablets->server->root
Old: `mem_tracker_OperationsFromDisk{}`
New: `mem_tracker_server_Tablets_PerTablet_OperationsFromDisk{}`

LogCache-f5b060aaca0041e0a8a5f0cbfc28dead->LogCache->server->root
Old: `mem_tracker_log_cache{}`
New: `mem_tracker_server_LogCache_PerLogCache{}`

/metrics from a tserver and master: [[ https://gist.github.com/yusong-yan/3ae57a5b55aabece3da4aaf586a0d9fe | github gist ]]

This diff also fixed the name issue that was introduced by D26876, where it modified the `server->root` MemTracker's metric name from `mem_tracker` to `root_server`.
Jira: DB-7554

Test Plan: Validated on dev server

Reviewers: asrivastava, rthallam, esheng

Reviewed By: rthallam

Subscribers: bogdan, ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D29433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.18 Backport Required area/docdb YugabyteDB core features kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

2 participants