abhishekagarwal87 commented on a change in pull request #11783:
URL: https://github.com/apache/druid/pull/11783#discussion_r724707831
##########
File path: docs/operations/metrics.md
##########
@@ -336,3 +336,17 @@ These metrics are only available if the SysMonitor module
is included.
|`sys/storage/used`|Disk space used.|fsDirName.|Varies.|
|`sys/cpu`|CPU used.|cpuName, cpuTime.|Varies.|
+## Cgroup
+
+These metrics are available on operating systems with the cgroup kernel
feature. All the values are derived by reading from `/sys/fs/cgroup`.
+
+|Metric|Description|Dimensions|Normal Value|
+|------|-----------|----------|------------|
+|`cgroup/cpu/shares`|Relative value of CPU time available to this process.
Read from `cpu.shares`.||Varies.|
+|`cgroup/cpu/cores_quota`|Number of cores available to this process. Derived
from `cpu.cfs_quota_us`/`cpu.cfs_period_us`.||Varies.|
Review comment:
can you add a note about does `-1` value for this metric implies?
##########
File path:
core/src/main/java/org/apache/druid/java/util/metrics/CgroupCpuMonitor.java
##########
@@ -65,12 +65,23 @@ public boolean doMonitor(ServiceEmitter emitter)
emitter.emit(builder.build("cgroup/cpu/shares", cpuSnapshot.getShares()));
emitter.emit(builder.build(
"cgroup/cpu/cores_quota",
- cpuSnapshot.getPeriodUs() == 0
- ? 0
- : ((double) cpuSnapshot.getQuotaUs()
- ) / cpuSnapshot.getPeriodUs()
+ computeProcessorQuota(cpuSnapshot.getQuotaUs(),
cpuSnapshot.getPeriodUs())
));
return true;
}
+
+ /**
+ * Calculates the total cores allocated through quotas.
+ *
+ * @param quotaUs the cgroup quota value.
+ * @param periodUs the cgroup period value.
+ * @return the calculated processor quota, -1 if no quota or period set.
+ */
+ public static double computeProcessorQuota(long quotaUs, long periodUs)
Review comment:
can you add some docs about why it should be `-1`?
##########
File path:
core/src/main/java/org/apache/druid/java/util/metrics/CgroupCpuMonitor.java
##########
@@ -65,12 +65,23 @@ public boolean doMonitor(ServiceEmitter emitter)
emitter.emit(builder.build("cgroup/cpu/shares", cpuSnapshot.getShares()));
emitter.emit(builder.build(
"cgroup/cpu/cores_quota",
- cpuSnapshot.getPeriodUs() == 0
- ? 0
- : ((double) cpuSnapshot.getQuotaUs()
- ) / cpuSnapshot.getPeriodUs()
+ computeProcessorQuota(cpuSnapshot.getQuotaUs(),
cpuSnapshot.getPeriodUs())
));
return true;
}
+
+ /**
+ * Calculates the total cores allocated through quotas.
+ *
+ * @param quotaUs the cgroup quota value.
+ * @param periodUs the cgroup period value.
+ * @return the calculated processor quota, -1 if no quota or period set.
+ */
+ public static double computeProcessorQuota(long quotaUs, long periodUs)
+ {
+ return quotaUs < 0 || periodUs == 0
Review comment:
can we use two values here? So if `quotaUs` < 0, we return `-1` and if
`periodUs` = 0, we return `-2`. This can be useful in troubleshooting. I
suppose the absolute value of these returned value is not going to be used
anywhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
