Add cache reservation logic by kotwanikunal · Pull Request #6350 · opensearch-project/OpenSearch

kotwanikunal · 2023-02-17T00:32:03Z

Description

This PR focuses on adding a reservation logic for the cache introduced as a part of searchable snapshots
The design was defined as a part of [Searchable Snapshot] Design file caching mechanism for block based files #4964 where a user can define the cache size using node.search.cache.size configuration for the node
This value is persisted as a part of the NodePath for the first data path configured on the node, and defaults to 100 MB otherwise.

Issues Resolved

Resolves [Searchable Snapshot] Implement disk reservation logic to be used for local file system cache #5508

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.

github-actions · 2023-02-17T00:52:02Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/11267/
CommitID: e72b498
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 · 2023-02-17T01:28:45Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/11271/
CommitID: e24ab81
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 · 2023-02-17T02:15:18Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/11276/
CommitID: 32cba61
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 · 2023-02-24T01:59:08Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/11687/
CommitID: 27e8b27
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 · 2023-02-24T03:09:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockIsRemovedWhenAnyNodesNotExceedHighWatermarkWithAutoReleaseEnabled

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

server/src/main/java/org/opensearch/env/NodeEnvironment.java

andrross · 2023-02-24T18:15:08Z

server/src/main/java/org/opensearch/index/IndexService.java

+      For a remote snapshot index, the cache path is used to initialize the shards.
+      For a local index, a local shard path is loaded or a new path is calculated.
+     */
+    private ShardPath getShardPath(ShardRouting routing, ShardId shardId, ShardLock lock) throws IOException {


What was the purpose of this refactoring? This is a private method that is only called from one place, so why break it into a separate method in this PR?

I did it for better readability. The initialization block was a lot of loading the shard path. This makes it easier to walk through that logic as a unit.

server/src/main/java/org/opensearch/monitor/fs/FsInfo.java

github-actions · 2023-02-24T20:13:50Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/11709/
CommitID: 4670c01
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 · 2023-02-24T20:13:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/11709/
CommitID: c7d7e70
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 · 2023-02-24T21:01:41Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/11710/
CommitID: a46ecba

github-actions · 2023-02-24T21:45:07Z

Gradle Check (Jenkins) Run Completed with:

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

andrross

One minor comment

server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

github-actions · 2023-02-24T22:15:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/11713/
CommitID: b409918

codecov-commenter · 2023-02-24T22:18:02Z

Codecov Report

Merging #6350 (b409918) into main (261f22c) will decrease coverage by 0.04%.
The diff coverage is 63.33%.

❗ Current head b409918 differs from pull request most recent head 60eb37f. Consider uploading reports for the commit 60eb37f to get more accurate results

📣 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    #6350      +/-   ##
============================================
- Coverage     70.76%   70.73%   -0.04%     
+ Complexity    59065    59062       -3     
============================================
  Files          4801     4801              
  Lines        282816   282901      +85     
  Branches      40773    40787      +14     
============================================
- Hits         200134   200100      -34     
- Misses        66273    66372      +99     
- Partials      16409    16429      +20

Impacted Files	Coverage Δ
...in/java/org/opensearch/indices/IndicesService.java	`62.99% <ø> (+0.25%)`	⬆️
.../java/org/opensearch/test/InternalTestCluster.java	`58.41% <12.50%> (+0.76%)`	⬆️
...rc/main/java/org/opensearch/monitor/fs/FsInfo.java	`72.13% <31.25%> (-2.87%)`	⬇️
...org/opensearch/common/settings/SettingsModule.java	`83.55% <40.00%> (-0.91%)`	⬇️
...src/main/java/org/opensearch/node/NodeService.java	`70.31% <50.00%> (+0.47%)`	⬆️
...c/main/java/org/opensearch/index/IndexService.java	`73.64% <62.50%> (-0.90%)`	⬇️
.../main/java/org/opensearch/env/NodeEnvironment.java	`75.21% <68.85%> (-0.96%)`	⬇️
server/src/main/java/org/opensearch/node/Node.java	`83.57% <75.00%> (-0.08%)`	⬇️
...c/main/java/org/opensearch/monitor/fs/FsProbe.java	`84.37% <90.00%> (+1.61%)`	⬆️
...ava/org/opensearch/cluster/node/DiscoveryNode.java	`91.80% <100.00%> (+0.59%)`	⬆️
... and 488 more

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-02-24T22:33:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/11715/
CommitID: 871f5e7

github-actions · 2023-02-24T22:40:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/11716/
CommitID: 60eb37f

opensearch-trigger-bot · 2023-02-24T22:57:03Z

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:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-6350-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 eb78246b93766e31a47112ab01872fc2d9052374
# Push it to GitHub
git push --set-upstream origin backport/backport-6350-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

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

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com> (cherry picked from commit eb78246)

kotwanikunal · 2023-02-25T00:02:06Z

Backport: #6485

* Include file cache stats in node stats response (#6333) This change exposes the file cache stats as part the node stats API response. Signed-off-by: Rabi Panda <adnapibar@gmail.com> (cherry picked from commit 0f3b870) * Add cache reservation logic (#6350) Signed-off-by: Kunal Kotwani <kkotwani@amazon.com> (cherry picked from commit eb78246) --------- Co-authored-by: Rabi Panda <adnapibar@gmail.com>

kotwanikunal added the skip-changelog label Feb 17, 2023

kotwanikunal requested review from Bukhtawar, CEHENKLE, Rishikesh1159, VachaShah, adnapibar, anasalkouz, andrross, dblock, dreamer-89, gbbafna, kartg, mch2, nknize, owaiskazi19, reta, ryanbogan, saratvemulapalli, setiah, shwetathareja, tlfeng and xuezhou25 as code owners February 17, 2023 00:32

kotwanikunal mentioned this pull request Feb 17, 2023

[Searchable Snapshots] Add integration test to validate cache files are closed after use #6215

Merged

6 tasks

kotwanikunal force-pushed the cache-reservation branch from e72b498 to e24ab81 Compare February 17, 2023 01:05

kotwanikunal force-pushed the cache-reservation branch from e24ab81 to 32cba61 Compare February 17, 2023 01:55

kotwanikunal added the backport 2.x Backport to 2.x branch label Feb 17, 2023

kotwanikunal force-pushed the cache-reservation branch from 32cba61 to 302a03a Compare February 17, 2023 02:13

kotwanikunal force-pushed the cache-reservation branch from 27e8b27 to de4048b Compare February 24, 2023 02:36

andrross mentioned this pull request Feb 24, 2023

[BUG] DiskThresholdDeciderIT.testIndexCreateBlockIsRemovedWhenAnyNodesNotExceedHighWatermarkWithAutoReleaseEnabled flaky test #6445

Closed

andrross reviewed Feb 24, 2023

View reviewed changes

kotwanikunal force-pushed the cache-reservation branch 2 times, most recently from 4670c01 to c7d7e70 Compare February 24, 2023 20:12

kotwanikunal force-pushed the cache-reservation branch from c7d7e70 to a46ecba Compare February 24, 2023 20:26

kotwanikunal force-pushed the cache-reservation branch 2 times, most recently from d59bc75 to b409918 Compare February 24, 2023 21:44

andrross reviewed Feb 24, 2023

View reviewed changes

server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java Show resolved Hide resolved

kotwanikunal force-pushed the cache-reservation branch from b409918 to 871f5e7 Compare February 24, 2023 22:00

Add cache reservation logic

60eb37f

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

kotwanikunal force-pushed the cache-reservation branch from 871f5e7 to 60eb37f Compare February 24, 2023 22:04

andrross approved these changes Feb 24, 2023

View reviewed changes

kotwanikunal merged commit eb78246 into opensearch-project:main Feb 24, 2023

kotwanikunal added a commit to kotwanikunal/OpenSearch that referenced this pull request Feb 24, 2023

Add cache reservation logic (opensearch-project#6350)

9354f73

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com> (cherry picked from commit eb78246)

kotwanikunal mentioned this pull request Feb 24, 2023

[Backport 2.x] Add file cache stats, reservation logic #6485

Merged

6 tasks

lukas-vlcek mentioned this pull request Apr 16, 2024

Display all file cache metrics in NodeStats and use the correct human-readable field name #13232

Open

4 tasks

Conversation

kotwanikunal commented Feb 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

github-actions bot commented Feb 17, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 17, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 17, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Uh oh!

Uh oh!

andrross Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

kotwanikunal Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

andrross left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

codecov-commenter commented Feb 24, 2023

Codecov Report

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Feb 24, 2023

Gradle Check (Jenkins) Run Completed with:

Uh oh!

opensearch-trigger-bot bot commented Feb 24, 2023

Uh oh!

kotwanikunal commented Feb 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kotwanikunal commented Feb 17, 2023 •

edited

Loading