You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/04/25 06:12:28 UTC
[GitHub] [pulsar] gaozhangmin opened a new pull request, #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
gaozhangmin opened a new pull request, #15304:
URL: https://github.com/apache/pulsar/pull/15304
<!--
### Contribution Checklist
- PR title format should be *[type][component] summary*. For details, see *[Guideline - Pulsar PR Naming Convention](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.trs9rsex3xom)*.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
Fixes #15229
### Motivation
conversion of NIC bandwidth and tx_bytes rx_bytes is wrong on linux platform.
### Modifications
For NIC bandwidth:
1,000 bit/s = 1 [kbit/s](https://en.wikipedia.org/wiki/Kbit/s) (one [thousand](https://en.wikipedia.org/wiki/Thousand) bits per second)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [ ] `no-need-doc`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-added`
(Docs have been already added)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1196243028
@gaozhangmin Could you migrate this PR to branch-2.8?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] erobot commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
erobot commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857577704
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(1024);
Review Comment:
https://github.com/apache/pulsar/blob/35c4b68f586774d0e2b7a3a2a6ab1d6a20ac1452/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LinuxBrokerHostUsageImpl.java#L114-L115
From this codes, usage and limit should have the same unit kbps so I thought this should be bit and usageBytes*8/1000.
By the way, UsageUnit.Kbps is somewhat misleading. It used to convert usageBytes but usageBytes's is size(byte) but not speed(kbps).
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857606446
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(1024);
Review Comment:
@erobot, Thanks for pointing it out, Because `totalNicLimit` is kbit. `nicUsageRx` and `nicUsageTx` should also be converted to kbit. `bytes*8/1000=kbit`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r858101214
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(8 / 1000);
Review Comment:
@michaeljmarshall It's my fault, I will change the enum name at another PR to make it easy to understand.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1136773938
Hi, @gaozhangmin
This PR looks could not cherry-pick to branch-2.9, could you mind pushing the new PR to branch-2.9?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 merged pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
Jason918 merged PR #15304:
URL: https://github.com/apache/pulsar/pull/15304
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857606446
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(1024);
Review Comment:
@erobot, Thanks for pointing it out, Because `totalNicLimit` is kbit. `nicUsageRx` and `nicUsageTx` should also be converted to kbit. so bytes*8/1000.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1109229220
Not in the scope of this PR, but related, see #15322
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1108118520
@gaozhangmin:Thanks for providing doc info!
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1148203642
@gaozhangmin
Hi, @gaozhangmin
This PR looks could not cherry-pick to branch-2.10, could you mind pushing the new PR to branch-2.10?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r858176834
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(8 / 1000);
Review Comment:
No worries, and thanks @mattisonchao!
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857891976
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(8 / 1000);
Review Comment:
Nit: this enum's name was confusing to me. After digging through the code, I can see that this enum's role is to convert the host's bytes in/out into kilobits in/out for comparison against the kilobits/sec rate. It could be valuable to either rename this enum or add a comment to the enum for future readers of the code.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857545483
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(1024);
Review Comment:
Yes, `tx_bytes` and `rx_bytes` unit is bytes, not bit. shouldn't divide 8
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857490327
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(1024);
Review Comment:
Is this unit related?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857545483
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(1024);
Review Comment:
Yes, `tx_bytes` and `rx_bytes` unit is bytes, not bite. shouldn't divide 8
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1197784421
I will cherry-pick https://github.com/apache/pulsar/pull/15770 instead.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1108117612
@gaozhangmin:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a diff in pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#discussion_r857545483
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java:
##########
@@ -254,7 +254,7 @@ public double convertBy(double usageBytes) {
@AllArgsConstructor
public enum UsageUnit {
- Kbps(8 / 1024);
+ Kbps(1024);
Review Comment:
Yes, `tx_bytes` and `rx_bytes` unit is bytes, not bit. shouldn't divide 8
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #15304: [fix][loadbalance] Fix wrong unit of NIC speed on linux
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on PR #15304:
URL: https://github.com/apache/pulsar/pull/15304#issuecomment-1108614348
@mattisonchao PTAL
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org