Skip to content

[fix][loadbalance] Fix wrong unit of NIC speed on linux - #15304

Merged
Jason918 merged 4 commits into
apache:masterfrom
gaozhangmin:fix-WrongNicSpeedConversionOnLinux
Apr 27, 2022
Merged

[fix][loadbalance] Fix wrong unit of NIC speed on linux#15304
Jason918 merged 4 commits into
apache:masterfrom
gaozhangmin:fix-WrongNicSpeedConversionOnLinux

Conversation

@gaozhangmin

@gaozhangmin gaozhangmin commented Apr 25, 2022 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

Fixes #15229

Motivation

conversion of NIC bandwidth and tx_bytes rx_bytes is wrong on linux platform.

Modifications

  1. For NIC bandwidth:
    1,000 bit/s = 1 kbit/s (one thousand bits per second)

  2. For tx_bytes and rx_bytes, the unit should same with NICLimit , so it should be bytes*8/1000=kbit rather than 8/1024.

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)

  • doc-not-needed

@github-actions

github-actions Bot commented Apr 25, 2022

Copy link
Copy Markdown

@gaozhangmin:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@gaozhangmin
gaozhangmin requested review from eolivelli and michaeljmarshall and removed request for eolivelli April 25, 2022 06:13
@gaozhangmin gaozhangmin added this to the 2.11.0 milestone Apr 25, 2022
@github-actions

github-actions Bot commented Apr 25, 2022

Copy link
Copy Markdown

@gaozhangmin:Thanks for providing doc info!

@github-actions github-actions Bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Apr 25, 2022
Jason918
Jason918 reviewed Apr 25, 2022
View reviewed changes
Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java Outdated
@gaozhangmin
gaozhangmin force-pushed the fix-WrongNicSpeedConversionOnLinux branch from 874822d to 499c560 Compare April 25, 2022 13:05
@gaozhangmin
gaozhangmin requested a review from Jason918 April 25, 2022 13:24
@gaozhangmin

gaozhangmin commented Apr 25, 2022

Copy link
Copy Markdown
Contributor Author

@mattisonchao PTAL

michaeljmarshall
michaeljmarshall approved these changes Apr 25, 2022
View reviewed changes

@michaeljmarshall michaeljmarshall left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I left a comment that is unrelated to this PR's changes.

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java Outdated
mattisonchao
mattisonchao approved these changes Apr 25, 2022
View reviewed changes

@mattisonchao mattisonchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, would you mind to push new PR to fix another branch? branch-2.8, branch-2.9, branch-2.10?

Jason918
Jason918 approved these changes Apr 26, 2022
View reviewed changes

@Jason918 Jason918 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Jason918

Jason918 commented Apr 26, 2022

Copy link
Copy Markdown
Contributor

Not in the scope of this PR, but related, see #15322

@Jason918
Jason918 merged commit 840723b into apache:master Apr 27, 2022
@gaozhangmin
gaozhangmin deleted the fix-WrongNicSpeedConversionOnLinux branch April 27, 2022 06:27
@mattisonchao

mattisonchao commented May 25, 2022

Copy link
Copy Markdown
Member

Hi, @gaozhangmin

This PR looks could not cherry-pick to branch-2.9, could you mind pushing the new PR to branch-2.9?

@gaozhangmin gaozhangmin mentioned this pull request May 25, 2022
Merged
@mattisonchao mattisonchao added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label May 30, 2022
@codelipenghui

codelipenghui commented Jun 7, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hi, @gaozhangmin

This PR looks could not cherry-pick to branch-2.10, could you mind pushing the new PR to branch-2.10?

@gaozhangmin gaozhangmin mentioned this pull request Jun 22, 2022
Merged
@BewareMyPower

BewareMyPower commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

@gaozhangmin Could you migrate this PR to branch-2.8?

@BewareMyPower

BewareMyPower commented Jul 28, 2022

Copy link
Copy Markdown
Contributor

I will cherry-pick #15770 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaeljmarshall michaeljmarshall michaeljmarshall approved these changes
@Jason918 Jason918 Jason918 approved these changes
@RobertIndie RobertIndie RobertIndie approved these changes
@mattisonchao mattisonchao mattisonchao approved these changes
@eolivelli eolivelli Awaiting requested review from eolivelli
@Technoboy- Technoboy- Awaiting requested review from Technoboy-
@lhotari lhotari Awaiting requested review from lhotari
+1 more reviewer
@erobot erobot erobot left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@gaozhangmin gaozhangmin

Labels

area/broker cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.9.3 release/2.10.2 type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Milestone

2.11.0

Development

Successfully merging this pull request may close these issues.

Wrong nic speed unit conversion on linux

9 participants

@gaozhangmin @Jason918 @mattisonchao @codelipenghui @BewareMyPower @erobot @RobertIndie @michaeljmarshall @Technoboy-