Fix flaky test S3BlobStoreRepositoryTests.testRequestStats #13887

msfroh · 2024-05-30T05:47:22Z

Description

If a BlobStoreRepository is created, but the blobStore() method is never called, then the actual blobStore instance won't be created. This means the repository will always emit non-detailed empty stats. When we would try to merge these stats with another repository that was initialized, it would fail the assertion that they either both have detailed stats or neither did.

This fix looks like it's checking if the blobStore has been initialized, but by calling blobStore(), it's making sure that it gets initialized.

Related Issues

Resolves #10735

Check List

~~New functionality includes testing.~~
- ~~All tests pass~~
~~New functionality has been documented.~~
- ~~New functionality has javadoc added~~
~~API changes companion pull request created.~~
~~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.

If a BlobStoreRepository is created, but the blobStore() method is never called, then the actual blobStore instance won't be created. This means the repository will always emit non-detailed empty stats. When we would try to merge these stats with another repository that was initialized, it would fail the assertion that they either both have detailed stats or neither did. This fix looks like it's checking if the blobStore has been initialized, but by calling blobStore(), it's making sure that it gets initialized. Signed-off-by: Michael Froh <froh@amazon.com>

github-actions · 2024-05-30T06:20:08Z

❌ Gradle check result for f08c62d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-05-30T07:07:52Z

❌ Gradle check result for f08c62d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

msfroh · 2024-05-30T07:34:47Z

I'm seeing test failures on org.opensearch.index.query.NestedQueryBuilderTests.testParentFilterFromInlineLeafInnerHitsNestedQuery which I think is a result of #13770 (which I merged just before opening this PR).

github-actions · 2024-05-30T08:19:15Z

❌ Gradle check result for f08c62d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

kkewwei · 2024-05-30T14:13:28Z

@msfroh I encountered in the #13853, I'm glad to see this pr.(I'm just plan to fix, If you don't mind, I'd like to describe the details.)

Because the BlobStoreRepository.blobStore is lazy initialized, it is initialized only when it is visited, two nodes will initial the BlobStoreRepository.blobStore.

one is master, it will be visited when the master process calling createSnapshot.
another is the node to which the shard is assigned), it will be visited when the data node starts to processing snapshot tasks.

There are another nodes, BlobStoreRepository.blobStore will not be visited, so RepositoryStats=RepositoryStats.EMPTY_STATS(detailed=false).

github-actions · 2024-05-30T16:56:39Z

❕ Gradle check result for f08c62d: UNSTABLE

TEST FAILURES:

      1 org.opensearch.cluster.MinimumClusterManagerNodesIT.testThreeNodesNoClusterManagerBlock

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

codecov · 2024-05-30T16:58:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.61%. Comparing base (b15cb0c) to head (f08c62d).
Report is 323 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #13887      +/-   ##
============================================
+ Coverage     71.42%   71.61%   +0.19%     
- Complexity    59978    61301    +1323     
============================================
  Files          4985     5064      +79     
  Lines        282275   288095    +5820     
  Branches      40946    41716     +770     
============================================
+ Hits         201603   206324    +4721     
- Misses        63999    64718     +719     
- Partials      16673    17053     +380

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

If a BlobStoreRepository is created, but the blobStore() method is never called, then the actual blobStore instance won't be created. This means the repository will always emit non-detailed empty stats. When we would try to merge these stats with another repository that was initialized, it would fail the assertion that they either both have detailed stats or neither did. This fix looks like it's checking if the blobStore has been initialized, but by calling blobStore(), it's making sure that it gets initialized. Signed-off-by: Michael Froh <froh@amazon.com> (cherry picked from commit c7e8421) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

msfroh · 2024-05-30T17:41:26Z

Ahh... Thanks @kkewwei !

I didn't know how we were hitting this condition, but that makes sense.

…13898) If a BlobStoreRepository is created, but the blobStore() method is never called, then the actual blobStore instance won't be created. This means the repository will always emit non-detailed empty stats. When we would try to merge these stats with another repository that was initialized, it would fail the assertion that they either both have detailed stats or neither did. This fix looks like it's checking if the blobStore has been initialized, but by calling blobStore(), it's making sure that it gets initialized. (cherry picked from commit c7e8421) Signed-off-by: Michael Froh <froh@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>

kkewwei · 2024-05-30T23:34:37Z

@msfroh I also find the same function(the content are same) OpenSearchMockAPIBasedRepositoryIntegTestCase.testRequestStats, I don't kno why it is same, and whether it should be fix?
https://github.com/opensearch-project/OpenSearch/blob/main/test/framework/src/main/java/org/opensearch/repositories/blobstore/OpenSearchMockAPIBasedRepositoryIntegTestCase.java#L190

…h-project#13887) If a BlobStoreRepository is created, but the blobStore() method is never called, then the actual blobStore instance won't be created. This means the repository will always emit non-detailed empty stats. When we would try to merge these stats with another repository that was initialized, it would fail the assertion that they either both have detailed stats or neither did. This fix looks like it's checking if the blobStore has been initialized, but by calling blobStore(), it's making sure that it gets initialized. Signed-off-by: Michael Froh <froh@amazon.com>

…h-project#13887) (opensearch-project#13898) If a BlobStoreRepository is created, but the blobStore() method is never called, then the actual blobStore instance won't be created. This means the repository will always emit non-detailed empty stats. When we would try to merge these stats with another repository that was initialized, it would fail the assertion that they either both have detailed stats or neither did. This fix looks like it's checking if the blobStore has been initialized, but by calling blobStore(), it's making sure that it gets initialized. (cherry picked from commit c7e8421) Signed-off-by: Michael Froh <froh@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> Signed-off-by: kkewwei <kkewwei@163.com>

…h-project#13887) If a BlobStoreRepository is created, but the blobStore() method is never called, then the actual blobStore instance won't be created. This means the repository will always emit non-detailed empty stats. When we would try to merge these stats with another repository that was initialized, it would fail the assertion that they either both have detailed stats or neither did. This fix looks like it's checking if the blobStore has been initialized, but by calling blobStore(), it's making sure that it gets initialized. Signed-off-by: Michael Froh <froh@amazon.com>

msfroh requested review from anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, dbwiddis, dreamer-89, gbbafna, kotwanikunal, mch2, nknize, owaiskazi19, reta, Rishikesh1159, sachinpkale, saratvemulapalli, shwetathareja, sohami, tlfeng and VachaShah as code owners May 30, 2024 05:47

github-actions bot added bug Something isn't working flaky-test Random test failure that succeeds on second run Storage Issues and PRs relating to data and metadata storage labels May 30, 2024

msfroh mentioned this pull request May 30, 2024

[BUG] org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testRequestStats is flaky #10735

Closed

msfroh added skip-changelog backport 2.x Backport to 2.x branch labels May 30, 2024

kkewwei mentioned this pull request May 30, 2024

add parameters to flat_object type #13853

Open

9 tasks

reta approved these changes May 30, 2024

View reviewed changes

reta merged commit c7e8421 into opensearch-project:main May 30, 2024
56 of 78 checks passed

opensearch-trigger-bot bot mentioned this pull request May 30, 2024

[Backport 2.x] Fix flaky test S3BlobStoreRepositoryTests.testRequestStats #13898

Merged

finnegancarroll mentioned this pull request Jun 14, 2024

Remove composeBuild fixture tasks when docker support not found #14357

Merged

3 tasks

prudhvigodithi mentioned this pull request Jun 18, 2024

[AUTOCUT] Gradle Check Flaky Test Report for IndicesRequestCacheIT prudhvigodithi/OpenSearch#27

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix flaky test S3BlobStoreRepositoryTests.testRequestStats #13887

Fix flaky test S3BlobStoreRepositoryTests.testRequestStats #13887

msfroh commented May 30, 2024 •

edited

Loading

github-actions bot commented May 30, 2024

github-actions bot commented May 30, 2024

msfroh commented May 30, 2024

github-actions bot commented May 30, 2024

kkewwei commented May 30, 2024 •

edited

Loading

github-actions bot commented May 30, 2024

codecov bot commented May 30, 2024

msfroh commented May 30, 2024

kkewwei commented May 30, 2024 •

edited

Loading

Fix flaky test S3BlobStoreRepositoryTests.testRequestStats #13887

Fix flaky test S3BlobStoreRepositoryTests.testRequestStats #13887

Conversation

msfroh commented May 30, 2024 • edited Loading

Description

Related Issues

Check List

github-actions bot commented May 30, 2024

github-actions bot commented May 30, 2024

msfroh commented May 30, 2024

github-actions bot commented May 30, 2024

kkewwei commented May 30, 2024 • edited Loading

github-actions bot commented May 30, 2024

codecov bot commented May 30, 2024

Codecov Report

msfroh commented May 30, 2024

kkewwei commented May 30, 2024 • edited Loading

msfroh commented May 30, 2024 •

edited

Loading

kkewwei commented May 30, 2024 •

edited

Loading

kkewwei commented May 30, 2024 •

edited

Loading