Skip to content

Conversation

FAWC438
Copy link
Contributor

@FAWC438 FAWC438 commented Apr 19, 2023

About high CPU usage bug discussed in issues#10672, my final solution was to set the init_wait parameter in the function decorator to 0.02 (20 ms). In addition, I also made additional changes to make the thread delay more reasonable and closer to the implementation of Java agent.

For the segment/log reporting function, I added a return value of bool type, which is only used by the @report_with_backoff decorator to determine whether the queue is not empty. If the queue is not empty, the thread will report the data immediately without any waiting; otherwise, the thread will wait for 20ms to release the CPU usage.

For other types of reporting functions, since their return value is always None, the function decorator still operates according to the original logic

I think the above changes balance the two solutions proposed by issues#10672 and make it more similar to the Java agent implementation

Fix

  • Add a unit test to verify that the fix works.
  • Explain briefly why the bug exists and how to fix it.
  • If this pull request closes/resolves/fixes an existing issue, replace the issue url. Closes: issues#10672
  • Update the CHANGELOG.md.

@report_with_backoff(reporter_name='segment', init_wait=0.02)
def __report_segment(self) -> bool:
"""Returns True if the queue is not empty"""
queue_not_empty_flag = not self.__segment_queue.empty()
Copy link

Choose a reason for hiding this comment

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

3% of developers fix this issue

reportOptionalMemberAccess: "empty" is not a known member of "None"


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? Answering this survey will not impact your Lift settings.

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@wu-sheng wu-sheng requested a review from Superskyyy April 19, 2023 05:09
@wu-sheng wu-sheng added the bug Something isn't working label Apr 19, 2023
@wu-sheng wu-sheng added this to the 1.1.0 milestone Apr 19, 2023
@Superskyyy
Copy link
Member

LGTM! I will kick off a release tomorrow if no objections.

@Superskyyy
Copy link
Member

@kezhenxu94 FYI, as you also encountered this problem in 1.0.0

@wu-sheng wu-sheng merged commit d2f74e0 into apache:master Apr 20, 2023
@wu-sheng
Copy link
Member

Are you going to cherry pick patches to do 1.0.1?
I think there is no block for this.

@Superskyyy
Copy link
Member

Yes, since its a patch version I will cherry pick them to do 1.0.1. Gonna figure out how to do that properly.

@wu-sheng
Copy link
Member

You should create 1.0.x branch based on 1.0.0 release commit. Then cherry pick this to that branch and upgrade version number for the release.
The main branch stays for 1.1.0 still.

@Superskyyy
Copy link
Member

You should create 1.0.x branch based on 1.0.0 release commit. Then cherry pick this to that branch and upgrade version number for the release. The main branch stays for 1.1.0 still.

I see, will do!

@FAWC438 FAWC438 deleted the fix_cpu_usage branch May 9, 2023 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants