Skip to content

[fix][broker] Fix the thread safety issue of BrokerData#getTimeAverageData access#19889

Merged
lordcheng10 merged 1 commit intoapache:masterfrom
lordcheng10:fix_BrokerData_update
Mar 27, 2023
Merged

[fix][broker] Fix the thread safety issue of BrokerData#getTimeAverageData access#19889
lordcheng10 merged 1 commit intoapache:masterfrom
lordcheng10:fix_BrokerData_update

Conversation

@lordcheng10
Copy link
Contributor

@lordcheng10 lordcheng10 commented Mar 22, 2023

Motivation

In the updateBundleData method, when the following code is executed:

// Using the newest data, update the aggregated time-average data for the current broker.
brokerData.getTimeAverageData().reset(statsMap.keySet(), bundleData, defaultStats);

In the reset method, information such as longTermMsgRateIn will be recalculated:
image

At this time, if brokerData.getTimeAverageData() is called in the LeastLongTermMessageRate#getScore method, the calculated score will be inaccurate:
image

Modifications

Create a new TimeAverageBrokerData object and perform a reset update. After the update is complete, call setTimeAverageData:

TimeAverageBrokerData timeAverageData = new TimeAverageBrokerData();
timeAverageData.reset(statsMap.keySet(), bundleData, defaultStats);
brokerData.setTimeAverageData(timeAverageData);

Documentation

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

Matching PR in forked repository

PR in forked repository: lordcheng10#45

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 22, 2023
@lordcheng10 lordcheng10 changed the title [fix][broker] Fix BrokerData#timeAverageData update is thread safe [fix][broker] Fix the thread safety issue of BrokerData#getTimeAverageData access Mar 22, 2023
Copy link
Contributor

@aloyszhang aloyszhang left a comment

Choose a reason for hiding this comment

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

nice catch, LGTM

@Technoboy- Technoboy- closed this Mar 24, 2023
@Technoboy- Technoboy- reopened this Mar 24, 2023
@lordcheng10 lordcheng10 merged commit 19c8497 into apache:master Mar 27, 2023
Technoboy- pushed a commit that referenced this pull request May 9, 2023
…eData access (#19889)

Co-authored-by: lordcheng10 <leolinchen@tencent.com>
nodece pushed a commit to nodece/pulsar that referenced this pull request Jul 23, 2024
…eData access (apache#19889)

Co-authored-by: lordcheng10 <leolinchen@tencent.com>
(cherry picked from commit 19c8497)
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Jul 24, 2024
…eData access (apache#19889)

Co-authored-by: lordcheng10 <leolinchen@tencent.com>
(cherry picked from commit 19c8497)
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