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

[fix][broker] usedLocallySinceLastReport should always be reset #22672

Merged
merged 2 commits into from
May 9, 2024

Conversation

nodece
Copy link
Member

@nodece nodece commented May 8, 2024

Motivation

org.apache.pulsar.broker.resourcegroup.ResourceGroup.PerMonitoringClassFields#usedLocallySinceLastReport holds the usage rate of ResourceGroup in the current cycle.

When the report thread calls the org.apache.pulsar.broker.resourcegroup.ResourceGroup#rgFillResourceUsage to report the ResourceGroup usage, the usedLocallySinceLastReport should be reset.

Right now, this value will be reported every 2 cycles. The correct behavior should be to report the current rate only when it exceeds or falls below 5% of the last reported value.

#22340 breaks this behavior.

Modifications

  • Always reset org.apache.pulsar.broker.resourcegroup.ResourceGroup.PerMonitoringClassFields#usedLocallySinceLastReport in the org.apache.pulsar.broker.resourcegroup.ResourceGroup#setUsageInMonitoredEntity.

Verifying this change

The test has been added.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@nodece nodece self-assigned this May 8, 2024
@nodece nodece added this to the 3.3.0 milestone May 8, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 8, 2024
@coderzc coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@nodece nodece closed this May 8, 2024
@nodece nodece reopened this May 8, 2024
Signed-off-by: Zixuan Liu <[email protected]>
@nodece nodece force-pushed the fix-usedLocallySinceLastReport-reset branch from e21f48d to 2388981 Compare May 8, 2024 10:55
@nodece nodece merged commit 8f015d8 into apache:master May 9, 2024
49 of 50 checks passed
nodece added a commit to nodece/pulsar that referenced this pull request May 10, 2024
nodece added a commit that referenced this pull request May 10, 2024
nodece added a commit that referenced this pull request May 10, 2024
nodece added a commit that referenced this pull request May 10, 2024
nodece added a commit that referenced this pull request May 10, 2024
@nodece nodece deleted the fix-usedLocallySinceLastReport-reset branch May 10, 2024 15:36
nodece added a commit to ascentstream/pulsar that referenced this pull request May 13, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 15, 2024
…he#22672)

Signed-off-by: Zixuan Liu <[email protected]>
(cherry picked from commit 8f015d8)
(cherry picked from commit 6b68a8d)
nodece added a commit to ascentstream/pulsar that referenced this pull request May 15, 2024
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 16, 2024
…he#22672)

Signed-off-by: Zixuan Liu <[email protected]>
(cherry picked from commit 8f015d8)
(cherry picked from commit 6b68a8d)
Technoboy- pushed a commit that referenced this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants