Skip to content

[fix] [broker] incorrect CPU usage collected by pulsar#17820

Closed
thetumbled wants to merge 10 commits into
apache:masterfrom
thetumbled:fixbug_incorrect_cpu_usage
Closed

[fix] [broker] incorrect CPU usage collected by pulsar#17820
thetumbled wants to merge 10 commits into
apache:masterfrom
thetumbled:fixbug_incorrect_cpu_usage

Conversation

@thetumbled

@thetumbled thetumbled commented Sep 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #17815

Incorrect CPU usage collected by pulsar

Motivation

cpu usage reported by pulsar is greater than 100%,needed to be fixed.

Modifications

org.apache.pulsar.broker.loadbalance.LinuxInfoUtils#getTotalCpuLimit
if CGROUP is enabled and quota is -1, we utilize the content of /sys/fs/cgroup/cpuset/cpuset.cpus to get the correct number of CPUs in the system.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Documentation

  • doc-not-needed

Matching PR in forked repository

PR in forked repository: thetumbled#1

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Sep 23, 2022
heesung-sohn
heesung-sohn reviewed Sep 26, 2022
View reviewed changes
Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java Outdated
heesung-sohn
heesung-sohn reviewed Oct 1, 2022
View reviewed changes

@heesung-sohn heesung-sohn left a comment •
edited
Loading

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.

please add a unit test for the parsing logic.

heesung-sohn
heesung-sohn reviewed Oct 1, 2022
View reviewed changes
Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java Outdated
@thetumbled

thetumbled commented Oct 1, 2022

Copy link
Copy Markdown
Member Author

add unit test and fix the bug already.

@heesung-sohn

heesung-sohn commented Oct 6, 2022

Copy link
Copy Markdown
Contributor

LGTM.

As per the new PR process, plz raise a PR on your fork repo and run the CI tests on your fork env first.(and link back the fork repo PR in this PR).

@thetumbled thetumbled mentioned this pull request Oct 12, 2022
Open
@thetumbled

thetumbled commented Oct 12, 2022

Copy link
Copy Markdown
Member Author

LGTM.

As per the new PR process, plz raise a PR on your fork repo and run the CI tests on your fork env first.(and link back the fork repo PR in this PR).

thetumbled#1

@hangc0276 hangc0276 added type/bug The PR fixed a bug or issue reported a bug release/2.9.4 release/2.10.3 labels Oct 26, 2022
@hangc0276 hangc0276 added this to the 2.11.0 milestone Oct 26, 2022
hangc0276
hangc0276 requested changes Oct 26, 2022
View reviewed changes
Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java
@congbobo184

congbobo184 commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

@thetumbled hi, I move this PR to release/2.9.5, if you have any questions, please ping me. thanks.

@thetumbled

thetumbled commented Nov 21, 2022

Copy link
Copy Markdown
Member Author

PTAL, thanks. @hangc0276

@github-actions

github-actions Bot commented Jan 12, 2023

Copy link
Copy Markdown

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions Bot added the Stale label Jan 12, 2023
@RobertIndie RobertIndie modified the milestones: 2.11.0, 3.1.0 Apr 11, 2023
@michaeljmarshall

michaeljmarshall commented Jun 27, 2023

Copy link
Copy Markdown
Member

As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label

@github-actions github-actions Bot removed the Stale label Jun 28, 2023
@Technoboy- Technoboy- modified the milestones: 3.1.0, 3.2.0 Jul 31, 2023
@thetumbled thetumbled changed the title [Bugfix] [LinuxInfoUtils] incorrect CPU usage collected by pulsar #17815 [fix] [broker] incorrect CPU usage collected by pulsar #17815 Sep 20, 2023
@thetumbled thetumbled changed the title [fix] [broker] incorrect CPU usage collected by pulsar #17815 [fix] [broker] incorrect CPU usage collected by pulsar Sep 20, 2023
@thetumbled

thetumbled commented Sep 21, 2023

Copy link
Copy Markdown
Member Author

PTAL, thanks. @hangc0276

heesung-sohn
heesung-sohn reviewed Sep 21, 2023
View reviewed changes
Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LinuxInfoUtils.java
if (quota > 0) {
return 100.0 * quota / period;
}
int totalCpuCount = getTotalCpuCount();

@heesung-sohn heesung-sohn Sep 21, 2023

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.

Actually, java 17 tracks container-aware resource usage. I think we just need to read jvm cpu limit and load if the jvm already provides the container-aware cpu limit and load.

Reference:
https://developers.redhat.com/articles/2022/04/19/java-17-whats-new-openjdks-container-awareness

@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023
@coderzc coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024
@lhotari

lhotari commented Oct 14, 2024

Copy link
Copy Markdown
Member

Addressed by #16832

@lhotari lhotari closed this Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@heesung-sohn heesung-sohn heesung-sohn left review comments
@hangc0276 hangc0276 Awaiting requested review from hangc0276

Assignees

@thetumbled thetumbled

Labels

doc-not-needed Your PR changes do not impact docs release/2.10.4 type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

[Bug] incorrect CPU usage collected by pulsar

10 participants

@thetumbled @heesung-sohn @congbobo184 @michaeljmarshall @lhotari @hangc0276 @Technoboy- @RobertIndie @coderzc @liangyepianzhou