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

org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testRequestStats fails with NullPointerException #13814

Merged
merged 1 commit into from
May 25, 2024

Conversation

akolarkunnu
Copy link
Contributor

@akolarkunnu akolarkunnu commented May 24, 2024

Description

It's a NullPointerException from S3BlobStore.extendedStats() method, where 'genericStatsMetricPublisher' is null. This parameter sets through S3Repository constructor from test and in this test it sets as null. This is the root cause of the issue. If we set valid a GenericStatsMetricPublisher, test works fine without any issue. This was a consistent failure, not a random failure.

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

Signed-off-by: Abdul Muneer Kolarkunnu [email protected]

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.

@dblock
Copy link
Member

dblock commented May 24, 2024

@akolarkunnu you need to git commit --amend -s for DCO to pass, please?

Copy link
Contributor

❌ Gradle check result for e8d3c47: 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?

…Stats fails with NullPointerException

It's a NullPointerException from S3BlobStore.extendedStats() method, where 'genericStatsMetricPublisher' is null. This parameter sets through S3Repository constructor from test and in this test it sets as null. This is the root cause of the issue. If we set valid a GenericStatsMetricPublisher, test works fine without any issue. This was a consistent failure, not a random failure.

Resolves opensearch-project#10735

Signed-off-by: akolarkunnu <[email protected]>
@akolarkunnu
Copy link
Contributor Author

akolarkunnu commented May 24, 2024

Known flaky org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"opensearch.experimental.feature.pluggable.caching.enabled":"false"}} - #13792
Known flaky org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"search.concurrent_segment_search.enabled":"false"}} - #13600
Known flaky org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"search.concurrent_segment_search.enabled":"true"}} - #13600
Known flaky org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"opensearch.experimental.feature.pluggable.caching.enabled":"true"}} - #13792

@akolarkunnu
Copy link
Contributor Author

akolarkunnu commented May 24, 2024

@dblock do I need to create fresh PR after running git commit --amend -s?

Also, since this is a test fix, how to skip Changelog Verifier?

@dblock
Copy link
Member

dblock commented May 24, 2024

@dblock do I need to create fresh PR after running git commit --amend -s?

No, you should git commit --amend -s and then force push the update git push origin testfix -f.

Also, since this is a test fix, how to skip Changelog Verifier?

I'll add the label.

@dblock
Copy link
Member

dblock commented May 24, 2024

org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"opensearch.experimental.feature.pluggable.caching.enabled":"false"}} org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"search.concurrent_segment_search.enabled":"false"}} org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"search.concurrent_segment_search.enabled":"true"}} org.opensearch.indices.IndicesRequestCacheIT.testDeleteAndCreateSameIndexShardOnSameNode {p0={"opensearch.experimental.feature.pluggable.caching.enabled":"true"}}

Link to the known issue for this so we can track flakes.

Copy link
Contributor

✅ Gradle check result for 1ba4adc: SUCCESS

Copy link

codecov bot commented May 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.65%. Comparing base (b15cb0c) to head (1ba4adc).
Report is 308 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13814      +/-   ##
============================================
+ Coverage     71.42%   71.65%   +0.23%     
- Complexity    59978    61353    +1375     
============================================
  Files          4985     5063      +78     
  Lines        282275   288027    +5752     
  Branches      40946    41710     +764     
============================================
+ Hits         201603   206388    +4785     
- Misses        63999    64644     +645     
- Partials      16673    16995     +322     

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

@akolarkunnu
Copy link
Contributor Author

@dblock Is it expecting 1 more approval?

@akolarkunnu
Copy link
Contributor Author

All checks are passed, @andrross @dblock Can you please merge this PR ?

@github-actions github-actions bot added bug Something isn't working flaky-test Random test failure that succeeds on second run good first issue Good for newcomers Storage Issues and PRs relating to data and metadata storage labels May 25, 2024
@jed326 jed326 added the backport 2.x Backport to 2.x branch label May 25, 2024
@jed326 jed326 merged commit 5cde176 into opensearch-project:main May 25, 2024
34 of 35 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-13814-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5cde176918744b9640d2ef84cf9ede4226950cea
# Push it to GitHub
git push --set-upstream origin backport/backport-13814-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-13814-to-2.x.

@jed326
Copy link
Collaborator

jed326 commented May 25, 2024

@akolarkunnu Looks would you mind opening a manual backport PR for 2.x branch?

@jed326 jed326 added the pending backport Identifies an issue or PR that still needs to be backported label May 26, 2024
@akolarkunnu
Copy link
Contributor Author

@akolarkunnu Looks would you mind opening a manual backport PR for 2.x branch?

Ok, let me try that.

@akolarkunnu
Copy link
Contributor Author

@jed326 I am getting below error while trying to push. This is my first checkin. Do I nee to get special permission to do backport?

remote: Permission to opensearch-project/OpenSearch.git denied to akolarkunnu.
fatal: unable to access 'https://github.com/opensearch-project/OpenSearch.git/': The requested URL returned error: 403

@jed326
Copy link
Collaborator

jed326 commented May 28, 2024

@akolarkunnu You will have to open a separate PR targeting the 2.x branch, you won't be able to push directly.

@akolarkunnu
Copy link
Contributor Author

@akolarkunnu You will have to open a separate PR targeting the 2.x branch, you won't be able to push directly.

I mean when I do this step:
git push --set-upstream origin backport/backport-13814-to-2.x

@andrross
Copy link
Member

@akolarkunnu I think the issue is that you need to be using your user fork when creating the PR so that in the above command 'origin' refers to your user fork.

@akolarkunnu
Copy link
Contributor Author

Backported - #13866

@akolarkunnu akolarkunnu deleted the testfix branch June 3, 2024 10:18
parv0201 pushed a commit to parv0201/OpenSearch that referenced this pull request Jun 10, 2024
…Stats fails with NullPointerException (opensearch-project#13814)

It's a NullPointerException from S3BlobStore.extendedStats() method, where 'genericStatsMetricPublisher' is null. This parameter sets through S3Repository constructor from test and in this test it sets as null. This is the root cause of the issue. If we set valid a GenericStatsMetricPublisher, test works fine without any issue. This was a consistent failure, not a random failure.

Resolves opensearch-project#10735

Signed-off-by: akolarkunnu <[email protected]>
Co-authored-by: akolarkunnu <[email protected]>
@akolarkunnu akolarkunnu restored the testfix branch July 9, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch backport-failed bug Something isn't working flaky-test Random test failure that succeeds on second run good first issue Good for newcomers pending backport Identifies an issue or PR that still needs to be backported skip-changelog Storage Issues and PRs relating to data and metadata storage
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[BUG] org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testRequestStats is flaky
4 participants