[Searchable Snapshot] Deleted the cache path on index deletion. #6830

harshjain2 · 2023-03-24T19:55:14Z

Description

Fix for the issue due to which cache path was not deleted when index is deleted.

Issues Resolved

Resolves #6532

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.

kotwanikunal · 2023-03-24T20:20:59Z

The solution LGTM. Are you working on adding in the test?

github-actions · 2023-03-24T20:30:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

URL: https://build.ci.opensearch.org/job/gradle-check/12896/
CommitID: c13bd58
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-commenter · 2023-03-24T20:33:34Z

Codecov Report

Merging #6830 (3f862b8) into main (07565ad) will increase coverage by 0.16%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #6830      +/-   ##
============================================
+ Coverage     70.65%   70.81%   +0.16%     
- Complexity    59162    59289     +127     
============================================
  Files          4812     4812              
  Lines        283615   283616       +1     
  Branches      40896    40896              
============================================
+ Hits         200394   200854     +460     
+ Misses        66764    66343     -421     
+ Partials      16457    16419      -38

Impacted Files	Coverage Δ
...index/store/remote/filecache/FileCacheCleaner.java	`44.44% <0.00%> (-2.62%)`	⬇️

... and 455 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

github-actions · 2023-03-25T09:47:00Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.index.IndexServiceTests.testAsyncTranslogTrimTaskOnClosedIndex

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

github-actions · 2023-03-25T10:24:36Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12911/
CommitID: 3f862b8

harshjain2 · 2023-03-25T19:08:23Z

@kotwanikunal Unit Tests doesn't exist for FileCacheCleaner.java probably because it is taking direct dependency on the Low Level Static APIs like Files. Therefore, I have modified the Integration Test case to cover this scenario.

Either the cache directory must be completely deleted. If not, the files in the cache directory must be properly closed. This is currently represented in the Integ Test case. Or should we change it to only first condition where the cache directory shouldn't exist at all?

harshjain2 · 2023-03-27T08:28:06Z

@kotwanikunal Please let me know if more changes are required for this PR.

github-actions · 2023-03-27T08:55:03Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/12943/
CommitID: 7b0fc4a

kotwanikunal · 2023-03-27T16:52:59Z

@harshjain2 LGTM. @andrross WDYT?

andrross

Either the cache directory must be completely deleted. If not, the files in the cache directory must be properly closed. This is currently represented in the Integ Test case. Or should we change it to only first condition where the cache directory shouldn't exist at all?

We deterministically delete the files when the index is deleted, therefore the case that the files exist but must be closed will never be encountered. I think we should remove that logic from the test case.

andrross · 2023-03-27T17:38:33Z

CHANGELOG.md

@@ -86,6 +86,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Segment Replication] Apply backpressure when replicas fall behind ([#6563](https://github.com/opensearch-project/OpenSearch/pull/6563))
 - [Remote Store] Integrate remote segment store in peer recovery flow ([#6664](https://github.com/opensearch-project/OpenSearch/pull/6664))
 - [Segment Replication] Add new cluster setting to set replication strategy by default for all indices in cluster. ([#6791](https://github.com/opensearch-project/OpenSearch/pull/6791))
+- [Searchable Snapshot] Deleted the cache path on index deletion. ([#6830](https://github.com/opensearch-project/OpenSearch/pull/6830))


We can omit the changelog here. We'll have a single over-arching entry for the feature and do not need these incremental fixes in the changelog.

Signed-off-by: Harsh Jain <harshjai@amazon.com>

harshjain2 · 2023-03-28T09:34:57Z

We deterministically delete the files when the index is deleted Taken this forward in the Integration Tests.

github-actions · 2023-03-28T10:03:04Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      2 org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=indices.split/30_copy_settings/Copy settings during split index}

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

harshjain2 · 2023-03-28T15:44:49Z

@andrross I have addressed the feedback. Let me know if you have any other feedback.

* [SearchableSnapshot] Deleted the cache path on index deletion. Signed-off-by: Harsh Jain <harshjai@amazon.com> * Added changelog, updated Integ Test. Signed-off-by: Harsh Jain <harshjai@amazon.com> * Removed changelog entry, updated validation in integ tests. Signed-off-by: Harsh Jain <harshjai@amazon.com> * Removed unused import statements. Signed-off-by: Harsh Jain <harshjai@amazon.com> --------- Signed-off-by: Harsh Jain <harshjai@amazon.com> Co-authored-by: Harsh Jain <harshjai@amazon.com> (cherry picked from commit bd9b00d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

… (#6856) * [SearchableSnapshot] Deleted the cache path on index deletion. * Added changelog, updated Integ Test. * Removed changelog entry, updated validation in integ tests. * Removed unused import statements. --------- (cherry picked from commit bd9b00d) Signed-off-by: Harsh Jain <harshjai@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: Harsh Jain <harshjai@amazon.com>

…n. (opensearch-project#6830) (opensearch-project#6856)" This reverts commit 2bb68aa.

…search-project#6830) * [SearchableSnapshot] Deleted the cache path on index deletion. Signed-off-by: Harsh Jain <harshjai@amazon.com> * Added changelog, updated Integ Test. Signed-off-by: Harsh Jain <harshjai@amazon.com> * Removed changelog entry, updated validation in integ tests. Signed-off-by: Harsh Jain <harshjai@amazon.com> * Removed unused import statements. Signed-off-by: Harsh Jain <harshjai@amazon.com> --------- Signed-off-by: Harsh Jain <harshjai@amazon.com> Co-authored-by: Harsh Jain <harshjai@amazon.com> Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>

github-actions bot added the first-time-contributor label Mar 24, 2023

kotwanikunal added the skip-changelog label Mar 24, 2023

harshjain2 changed the title ~~[SearchableSnapshot] Deleted the cache path on index deletion.~~ [Searchable Snapshot] Deleted the cache path on index deletion. Mar 25, 2023

harshjain2 force-pushed the harshjain2/cache_path_deletion branch from 8cfcbfe to 3f862b8 Compare March 25, 2023 09:57

harshjain2 marked this pull request as ready for review March 25, 2023 19:03

harshjain2 requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng and VachaShah as code owners March 25, 2023 19:03

harshjain2 requested a review from xuezhou25 as a code owner March 25, 2023 19:04

harshjain2 force-pushed the harshjain2/cache_path_deletion branch from 3f862b8 to 7b0fc4a Compare March 27, 2023 08:27

kotwanikunal added the backport 2.x Backport to 2.x branch label Mar 27, 2023

kotwanikunal approved these changes Mar 27, 2023

View reviewed changes

andrross reviewed Mar 27, 2023

View reviewed changes

Harsh Jain added 4 commits March 28, 2023 15:01

[SearchableSnapshot] Deleted the cache path on index deletion.

6d8c1a2

Signed-off-by: Harsh Jain <harshjai@amazon.com>

Added changelog, updated Integ Test.

ffcf6d9

Signed-off-by: Harsh Jain <harshjai@amazon.com>

Removed changelog entry, updated validation in integ tests.

6a30bf6

Signed-off-by: Harsh Jain <harshjai@amazon.com>

Removed unused import statements.

e42b83b

Signed-off-by: Harsh Jain <harshjai@amazon.com>

harshjain2 force-pushed the harshjain2/cache_path_deletion branch from 7b0fc4a to e42b83b Compare March 28, 2023 09:34

andrross approved these changes Mar 28, 2023

View reviewed changes

andrross merged commit bd9b00d into opensearch-project:main Mar 28, 2023

opensearch-trigger-bot bot mentioned this pull request Mar 28, 2023

[Backport 2.x] [Searchable Snapshot] Deleted the cache path on index deletion. #6856

Merged

Rishikesh1159 added a commit to Rishikesh1159/OpenSearch that referenced this pull request Mar 30, 2023

Revert "[Searchable Snapshot] Deleted the cache path on index deletio…

c1b93b3

…n. (opensearch-project#6830) (opensearch-project#6856)" This reverts commit 2bb68aa.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Searchable Snapshot] Deleted the cache path on index deletion. #6830

[Searchable Snapshot] Deleted the cache path on index deletion. #6830

harshjain2 commented Mar 24, 2023 •

edited

Loading

kotwanikunal commented Mar 24, 2023

github-actions bot commented Mar 24, 2023

codecov-commenter commented Mar 24, 2023 •

edited

Loading

github-actions bot commented Mar 25, 2023

github-actions bot commented Mar 25, 2023

harshjain2 commented Mar 25, 2023

harshjain2 commented Mar 27, 2023

github-actions bot commented Mar 27, 2023

kotwanikunal commented Mar 27, 2023

andrross left a comment

andrross Mar 27, 2023

harshjain2 Mar 28, 2023

harshjain2 commented Mar 28, 2023

github-actions bot commented Mar 28, 2023

harshjain2 commented Mar 28, 2023

[Searchable Snapshot] Deleted the cache path on index deletion. #6830

[Searchable Snapshot] Deleted the cache path on index deletion. #6830

Conversation

harshjain2 commented Mar 24, 2023 • edited Loading

Description

Issues Resolved

Check List

kotwanikunal commented Mar 24, 2023

github-actions bot commented Mar 24, 2023

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Mar 24, 2023 • edited Loading

Codecov Report

github-actions bot commented Mar 25, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 25, 2023

Gradle Check (Jenkins) Run Completed with:

harshjain2 commented Mar 25, 2023

harshjain2 commented Mar 27, 2023

github-actions bot commented Mar 27, 2023

Gradle Check (Jenkins) Run Completed with:

kotwanikunal commented Mar 27, 2023

andrross left a comment

Choose a reason for hiding this comment

andrross Mar 27, 2023

Choose a reason for hiding this comment

harshjain2 Mar 28, 2023

Choose a reason for hiding this comment

harshjain2 commented Mar 28, 2023

github-actions bot commented Mar 28, 2023

Gradle Check (Jenkins) Run Completed with:

harshjain2 commented Mar 28, 2023

harshjain2 commented Mar 24, 2023 •

edited

Loading

codecov-commenter commented Mar 24, 2023 •

edited

Loading