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

[Backport 2.14] [Bugfix] Fix flaky test CacheStatsAPIIndicesRequestCacheIT.testNullLevels() #13485

Merged

Conversation

peteralfonsi
Copy link
Contributor

Original PR: #13457

2.x Backport PR: #13475

Description

I introduced CacheStatsAPIIndicesRequestCacheIT in this recent PR but I have found it to be flaky (example here). I created an issue for this here.

If levels is uninitialized in CommonStatsFlags, the String[] levels passed into DefaultCacheStatsHolder is usually an empty list. This is the expected behavior. But in some cases, DefaultCacheStatsHolder receives null instead of an empty list, and in this case, the XContent output derived from it has unexpected fields. To fix this we add an explicit null check when DefaultCacheStatsHolder constructs the object which produces XContent, and use an empty list if null is passed in.

We also initialize CommonStatsFlags.levels to be an empty String[] rather than null, as I found not doing this can rarely cause RestNodesStatsActionTests.testUnrecognizedMetric() to flake for the same reason.

Finally, fixes inconsistency with how null levels were used in tests.

Related Issues

Resolves #13458

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…pensearch-project#13457) (opensearch-project#13475)

* Fix flaky test

* Initialize CommonStatsFlags with empty array for levels

* Fixes tests using incorrect null levels

* rerun

---------

(cherry picked from commit 5d61ac2)

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>
(cherry picked from commit 291f686)
Copy link
Contributor

github-actions bot commented May 1, 2024

❕ Gradle check result for af16613: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.indices.replication.SegmentReplicationIT.testReplicaAlreadyAtCheckpoint
      1 org.opensearch.action.admin.indices.create.CreateIndexIT.testCreateAndDeleteIndexConcurrently
      1 org.opensearch.action.admin.indices.create.CreateIndexIT.classMethod

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented May 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (2.14@b8940cf). Click here to learn what that means.

Additional details and impacted files
@@           Coverage Diff           @@
##             2.14   #13485   +/-   ##
=======================================
  Coverage        ?   71.19%           
  Complexity      ?    61148           
=======================================
  Files           ?     5026           
  Lines           ?   287473           
  Branches        ?    42017           
=======================================
  Hits            ?   204675           
  Misses          ?    65607           
  Partials        ?    17191           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dblock dblock merged commit 8e786c2 into opensearch-project:2.14 May 1, 2024
51 of 52 checks passed
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.

2 participants