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

[CELEBORN-1706] Use bytes(IEC) unit instead of bytes(SI) for size related metrics in prometheus dashboard #2896

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

turboFei
Copy link
Member

@turboFei turboFei commented Nov 10, 2024

What changes were proposed in this pull request?

Use unit bytes(IEC)(decbytes, 1,024 bytes in a kibibyte ) for below 18 metrics(disk and memory related) instead of bytes(SI)(bytes, 1,000 bytes in a kilobyte).

  • metrics_DeviceCelebornTotalBytes_Value
  • metrics_DeviceCelebornFreeBytes_Value
  • metrics_PartitionSize_Value
  • metrics_ActiveShuffleSize_Value
  • metrics_NettyMemory_Value
  • metrics_DiskBuffer_Value
  • metrics_push_usedHeapMemory_Value
  • metrics_push_usedDirectMemory_Value
  • metrics_fetch_usedHeapMemory_Value
  • metrics_fetch_usedDirectMemory_Value
  • metrics_replicate_usedHeapMemory_Value
  • metrics_replicate_usedDirectMemory_Value
  • metrics_BufferStreamReadBuffer_Value
  • metrics_SortMemory_Value
  • metrics_DeviceOSFreeBytes_Value
  • metrics_DeviceCelebornFreeBytes_Value
  • metrics_diskBytesWritten_Value
  • metrics_hdfsBytesWritten_Value

Also apply for 6 jvm metrics

  • metrics_jvm_memory_heap_init_Value
  • metrics_jvm_memory_non_heap_init_Value
  • metrics_jvm_memory_total_init_Value
  • metrics_jvm_memory_pools_init_Value
  • metrics_jvm_direct_capacity_Value
  • metrics_jvm_mapped_capacity_Value

Why are the changes needed?

Some size related metrics use bytes(IEC) and some use bytes(SI).
image

image

The main difference between bytes in the International System of Units (SI) and the International Electrotechnical Commission (IEC) is the number of bytes in a kilobyte:
SI: 1,000 bytes in a kilobyte
IEC: 1,024 bytes in a kibibyte

FYI: https://www.drupal.org/project/drupal/issues/1114538#:~:text=According%20to%20the%20SI%20standard,e.g.%20a%20stick%20of%20RAM.

"expr": "metrics_DirectMemoryUsageRatio_Value{instance=~\"${instance}\"}",
"legendFormat": "${baseLegend}",
"instant": false,
"range": true,
"refId": "A"
}
],
"title": "DirectMemoryUsageRatio",
"type": "timeseries"
},
{
"datasource": {
"type": "prometheus",
"uid": "${DS_PROMETHEUS}"
},
"fieldConfig": {
"defaults": {
"color": {
"mode": "palette-classic"
},
"custom": {
"axisCenteredZero": false,
"axisColorMode": "text",
"axisLabel": "",
"axisPlacement": "auto",
"barAlignment": 0,
"drawStyle": "line",
"fillOpacity": 0,
"gradientMode": "none",
"hideFrom": {
"legend": false,
"tooltip": false,
"viz": false
},
"lineInterpolation": "linear",
"lineWidth": 1,
"pointSize": 5,
"scaleDistribution": {
"type": "linear"
},
"showPoints": "auto",
"spanNulls": false,
"stacking": {
"group": "A",
"mode": "none"
},
"thresholdsStyle": {
"mode": "off"
}
},
"mappings": [],
"thresholds": {
"mode": "absolute",
"steps": [
{
"color": "green"
},
{
"color": "red",
"value": 80
}
]
},
"unit": "decbytes"

Does this PR introduce any user-facing change?

Yes, metrics unit changed.

How was this patch tested?

Not needed, we already use decbytes in the dashboard json.

@turboFei turboFei changed the title use unit decbytes [CELEBORN-1706] Use decbytes unit for size related metrics Nov 10, 2024
@turboFei turboFei changed the title [CELEBORN-1706] Use decbytes unit for size related metrics [CELEBORN-1706] Use decbytes unit for size related metrics in prometheus dashboard Nov 10, 2024
@turboFei turboFei changed the title [CELEBORN-1706] Use decbytes unit for size related metrics in prometheus dashboard [CELEBORN-1706] Use bytes(IEC) unit instead of bytes(SI) for size related metrics in prometheus dashboard Nov 10, 2024
Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea lgtm, and do we have the contribution guide for modifying the grafana dashboard template? we should emphasis it there.

@turboFei
Copy link
Member Author

do we have the contribution guide for modifying the grafana dashboard template?

No, currently it is a little difficult to add a new metrics.

Copy link
Contributor

@FMX FMX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know there was a difference between the SI and IEC. Thanks. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants