Make sure IndexOutput of metadata file is always closed #8653

sachinpkale · 2023-07-12T01:41:19Z

Description

Backporting part of Fix remote segments sync retry regression introduced in PR #7119 #8632 to fix the flaky tests. We can't backport Fix remote segments sync retry regression introduced in PR #7119 #8632 completely as it is dependent on another PR.

Related Issues

Resolves #8638

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

Signed-off-by: Sachin Kale <[email protected]>

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java

github-actions · 2023-07-12T02:19:04Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.search.SearchWeightedRoutingIT.testStrictWeightedRoutingWithCustomString
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testRestartPrimary
      1 org.opensearch.indices.replication.SegmentReplicationRelocationIT.testRelocateWhileContinuouslyIndexingAndWaitingForRefresh
      1 org.opensearch.action.admin.cluster.node.tasks.ResourceAwareTasksTests.testBasicTaskResourceTracking

URL: https://build.ci.opensearch.org/job/gradle-check/19997/
CommitID: 3b9f11a
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 · 2023-07-12T02:19:46Z

Codecov Report

Merging #8653 (792cc27) into 2.x (8c18f51) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                2.x    #8653      +/-   ##
============================================
- Coverage     70.71%   70.69%   -0.02%     
+ Complexity    57178    57157      -21     
============================================
  Files          4745     4745              
  Lines        270757   270756       -1     
  Branches      39961    39961              
============================================
- Hits         191456   191424      -32     
- Misses        62850    62875      +25     
- Partials      16451    16457       +6

Impacted Files	Coverage Δ
...earch/index/store/RemoteSegmentStoreDirectory.java	`92.05% <100.00%> (+2.75%)`	⬆️

... and 457 files with indirect coverage changes

dreamer-89 · 2023-07-12T02:26:00Z

Let's have another pair of eyes for review as we want this in 2.9 branch. @mch2 @ashking94 @gbbafna @Bukhtawar : Ping for review

Signed-off-by: Sachin Kale <[email protected]>

ashking94 · 2023-07-12T02:55:17Z

Let's have another pair of eyes for review as we want this in 2.9 branch. @mch2 @ashking94 @gbbafna @Bukhtawar : Ping for review

@dreamer-89 The changes looks good.

github-actions · 2023-07-12T03:18:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/20000/
CommitID: 792cc27
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?

sachinpkale · 2023-07-12T03:38:02Z

Checking the build failure.

github-actions · 2023-07-12T04:34:07Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.indices.replication.RemoteStoreReplicationSourceTests.testGetSegmentFilesFailure

URL: https://build.ci.opensearch.org/job/gradle-check/20006/
CommitID: 792cc27
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

…roject#8653) --------- Signed-off-by: Sachin Kale <[email protected]> Co-authored-by: Sachin Kale <[email protected]>

--------- Signed-off-by: Sachin Kale <[email protected]> Co-authored-by: Sachin Kale <[email protected]>

dreamer-89 · 2023-08-03T17:30:14Z

server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java

@@ -604,6 +604,28 @@ public void testUploadMetadataNonEmpty() throws IOException {
        }
    }

+    public void testUploadMetadataMissingSegment() throws IOException {


@sachinpkale : This test is missing in main. This needs authors to fix/amend this test separately on 2.x. Example PR #9076

this test is now failing on 2.x - #9095

This is fixed with #9099

Make sure IndexOutput of metadata file is always closed

3b9f11a

Signed-off-by: Sachin Kale <[email protected]>

sachinpkale requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and dbwiddis as code owners July 12, 2023 01:41

sachinpkale added the skip-changelog label Jul 12, 2023

dreamer-89 reviewed Jul 12, 2023

View reviewed changes

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java Show resolved Hide resolved

dreamer-89 approved these changes Jul 12, 2023

View reviewed changes

Add unit test

792cc27

Signed-off-by: Sachin Kale <[email protected]>

ashking94 approved these changes Jul 12, 2023

View reviewed changes

linuxpi approved these changes Jul 12, 2023

View reviewed changes

gbbafna approved these changes Jul 12, 2023

View reviewed changes

sachinpkale merged commit 25a6a82 into opensearch-project:2.x Jul 12, 2023

sachinpkale mentioned this pull request Jul 12, 2023

Make sure IndexOutput of metadata file is always closed (#8653) #8656

Merged

sachinpkale added a commit that referenced this pull request Jul 12, 2023

Make sure IndexOutput of metadata file is always closed (#8653) (#8656)

9087d05

--------- Signed-off-by: Sachin Kale <[email protected]> Co-authored-by: Sachin Kale <[email protected]>

dreamer-89 reviewed Aug 3, 2023

View reviewed changes

dreamer-89 mentioned this pull request Aug 3, 2023

[Remote Store] Add missing unit test in main #9101

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sure IndexOutput of metadata file is always closed #8653

Make sure IndexOutput of metadata file is always closed #8653

sachinpkale commented Jul 12, 2023 •

edited

Loading

github-actions bot commented Jul 12, 2023

codecov bot commented Jul 12, 2023 •

edited

Loading

dreamer-89 commented Jul 12, 2023

ashking94 commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

sachinpkale commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

dreamer-89 Aug 3, 2023

mch2 Aug 3, 2023

dreamer-89 Aug 3, 2023

Make sure IndexOutput of metadata file is always closed #8653

Make sure IndexOutput of metadata file is always closed #8653

Conversation

sachinpkale commented Jul 12, 2023 • edited Loading

Description

Related Issues

Check List

github-actions bot commented Jul 12, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Jul 12, 2023 • edited Loading

Codecov Report

dreamer-89 commented Jul 12, 2023

ashking94 commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

Gradle Check (Jenkins) Run Completed with:

sachinpkale commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

Gradle Check (Jenkins) Run Completed with:

dreamer-89 Aug 3, 2023

Choose a reason for hiding this comment

mch2 Aug 3, 2023

Choose a reason for hiding this comment

dreamer-89 Aug 3, 2023

Choose a reason for hiding this comment

sachinpkale commented Jul 12, 2023 •

edited

Loading

codecov bot commented Jul 12, 2023 •

edited

Loading