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

[PROF-10422] Fix flaky GVL profiling integration spec #3936

Merged
merged 1 commit into from
Sep 20, 2024

Commits on Sep 20, 2024

  1. [PROF-10422] Fix flaky GVL profiling integration spec

    **What does this PR do?**
    
    This PR fixes a flaky specs seen in
    https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/16487/workflows/df69cfde-1fd5-40dc-bff6-ac8a9d0bc713/jobs/593534 :
    
    ```
      1) Datadog::Profiling::Collectors::CpuAndWallTimeWorker#start when main thread is sleeping but a background thread is working when GVL profiling is enabled records Waiting for GVL samples
         Failure/Error: expect(waiting_for_gvl_time).to be_within(5).percent_of(total_time)
           expected 402832467 to be within 5% of 492191168
    ```
    
    I was able to reproduce this failure on my local machine in around 1 in
    20 runs.
    
    This flakiness is caused by the profiler starting after a Waiting for
    GVL already started, and thus the profiler does not see it and still
    categorizes that period as "unknown"; thus our tight assertion is blown.
    
    It's not a concidence that the difference between 492191168 and
    402832467 is within ~100ms: that's the scheduler latency caused
    by the background thread that's burning CPU.
    
    To fix this issue, we subtract the duration of this initial period
    from the `total_time`: this is the period we care for.
    
    To avoid us accidentally passing this test by having no Waiting for GVL,
    I've added an additional assertion for a sane `total_value`.
    
    **Motivation:**
    
    Make sure the profiler has zero flaky specs.
    
    **Additional Notes:**
    
    N/A
    
    **How to test the change?**
    
    Validate that CI is green and that the flaky test does not show up
    again.
    ivoanjo committed Sep 20, 2024
    Configuration menu
    Copy the full SHA
    be8882a View commit details
    Browse the repository at this point in the history