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

[Writable Warm] Composite Directory implementation and integrating it with FileCache #12782

Merged
merged 22 commits into from
Jun 20, 2024

Conversation

rayshrey
Copy link
Contributor

@rayshrey rayshrey commented Mar 20, 2024

Description

Implements and design Composite Directory (and integrates it with FileCache) which can fall back to the on-demand fetch when requested data is not present locally.

Opening this PR to get initial review comments on the design and implementation of Composite Directory in this issue - #12781, hence haven't added any tests apart from one IT test for testing the basic functionality. Will include tests in the following revisions to this PR.

Related Issues

Resolves #[12781]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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.

Copy link
Contributor

github-actions bot commented Mar 20, 2024

Compatibility status:

Checks if related components are compatible with change f442533

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/sql.git]

Copy link
Contributor

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

rayshrey added 6 commits June 20, 2024 12:56
…or ref count via cloneMap in FullFileCachedIndexInput and other review fixes

Signed-off-by: Shreyansh Ray <[email protected]>
Signed-off-by: Shreyansh Ray <[email protected]>
Signed-off-by: Shreyansh Ray <[email protected]>
@rayshrey rayshrey force-pushed the composite-directory-poc branch from 57eca38 to 901849b Compare June 20, 2024 07:26
@rayshrey
Copy link
Contributor Author

rayshrey commented Jun 20, 2024

Flaky tests causing build to fail intermittently over the past week

#9464
#14319
#14304
#14288
#14291

Copy link
Contributor

❕ Gradle check result for 901849b: UNSTABLE

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

@sohami
Copy link
Collaborator

sohami commented Jun 20, 2024

❕ Gradle check result for 901849b: UNSTABLE

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

#14291

@sohami sohami merged commit b8c7819 into opensearch-project:main Jun 20, 2024
31 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-12782-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b8c78196438897132f6819460ebb7d4222b39297
# Push it to GitHub
git push --set-upstream origin backport/backport-12782-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-12782-to-2.x.

@sohami
Copy link
Collaborator

sohami commented Jun 20, 2024

@rayshrey Can you please create a manual backport PR for 2.x as above failed

rayshrey added a commit to rayshrey/OpenSearch that referenced this pull request Jun 20, 2024
… with FileCache (opensearch-project#12782)

* Composite Directory POC

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor TransferManager interface to RemoteStoreFileTrackerAdapter

Signed-off-by: Shreyansh Ray <[email protected]>

* Implement block level fetch for Composite Directory

Signed-off-by: Shreyansh Ray <[email protected]>

* Removed CACHE state from FileTracker

Signed-off-by: Shreyansh Ray <[email protected]>

* Fixes after latest pull

Signed-off-by: Shreyansh Ray <[email protected]>

* Add new setting for warm, remove store type setting, FileTracker and RemoteStoreFileTrackerAdapter, CompositeDirectoryFactory and update Composite Directory implementation

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify TransferManager - replace BlobContainer with Functional Interface to fetch an InputStream instead

Signed-off-by: Shreyansh Ray <[email protected]>

* Reuse OnDemandBlockSnapshotIndexInput instead of OnDemandBlockCompositeIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify constructors to avoid breaking public api contract and code review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add experimental annotations for newly created classes and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Use ref count as a temporary measure to prevent file from eviction until uploaded to Remote

Signed-off-by: Shreyansh Ray <[email protected]>

* Remove method level locks

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle tmp file deletion

Signed-off-by: Shreyansh Ray <[email protected]>

* Nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle delete and close in Composite Directory, log current state of FileCache and correct it's clear method and modify unit and integration tests as per review comments

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor usages of WRITEABLE_REMOTE_INDEX_SETTING to TIERED_REMOTE_INDEX_SETTING

Signed-off-by: Shreyansh Ray <[email protected]>

* Add tests for FileCachedIndexInput and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add additional IT for feature flag disabled

Signed-off-by: Shreyansh Ray <[email protected]>

* Move setting for Partial Locality type behind Feature Flag, fix bug for ref count via cloneMap in FullFileCachedIndexInput and other review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor test and nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add javadocs for FullFileCachedIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor precommit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>
rayshrey added a commit to rayshrey/OpenSearch that referenced this pull request Jun 21, 2024
… with FileCache (opensearch-project#12782)

* Composite Directory POC

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor TransferManager interface to RemoteStoreFileTrackerAdapter

Signed-off-by: Shreyansh Ray <[email protected]>

* Implement block level fetch for Composite Directory

Signed-off-by: Shreyansh Ray <[email protected]>

* Removed CACHE state from FileTracker

Signed-off-by: Shreyansh Ray <[email protected]>

* Fixes after latest pull

Signed-off-by: Shreyansh Ray <[email protected]>

* Add new setting for warm, remove store type setting, FileTracker and RemoteStoreFileTrackerAdapter, CompositeDirectoryFactory and update Composite Directory implementation

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify TransferManager - replace BlobContainer with Functional Interface to fetch an InputStream instead

Signed-off-by: Shreyansh Ray <[email protected]>

* Reuse OnDemandBlockSnapshotIndexInput instead of OnDemandBlockCompositeIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify constructors to avoid breaking public api contract and code review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add experimental annotations for newly created classes and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Use ref count as a temporary measure to prevent file from eviction until uploaded to Remote

Signed-off-by: Shreyansh Ray <[email protected]>

* Remove method level locks

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle tmp file deletion

Signed-off-by: Shreyansh Ray <[email protected]>

* Nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle delete and close in Composite Directory, log current state of FileCache and correct it's clear method and modify unit and integration tests as per review comments

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor usages of WRITEABLE_REMOTE_INDEX_SETTING to TIERED_REMOTE_INDEX_SETTING

Signed-off-by: Shreyansh Ray <[email protected]>

* Add tests for FileCachedIndexInput and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add additional IT for feature flag disabled

Signed-off-by: Shreyansh Ray <[email protected]>

* Move setting for Partial Locality type behind Feature Flag, fix bug for ref count via cloneMap in FullFileCachedIndexInput and other review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor test and nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add javadocs for FullFileCachedIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor precommit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>
sohami pushed a commit that referenced this pull request Jun 24, 2024
… integrating it with FileCache (#14489)

* [Writable Warm] Composite Directory implementation and integrating it with FileCache (#12782)

* Composite Directory POC

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor TransferManager interface to RemoteStoreFileTrackerAdapter

Signed-off-by: Shreyansh Ray <[email protected]>

* Implement block level fetch for Composite Directory

Signed-off-by: Shreyansh Ray <[email protected]>

* Removed CACHE state from FileTracker

Signed-off-by: Shreyansh Ray <[email protected]>

* Fixes after latest pull

Signed-off-by: Shreyansh Ray <[email protected]>

* Add new setting for warm, remove store type setting, FileTracker and RemoteStoreFileTrackerAdapter, CompositeDirectoryFactory and update Composite Directory implementation

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify TransferManager - replace BlobContainer with Functional Interface to fetch an InputStream instead

Signed-off-by: Shreyansh Ray <[email protected]>

* Reuse OnDemandBlockSnapshotIndexInput instead of OnDemandBlockCompositeIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify constructors to avoid breaking public api contract and code review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add experimental annotations for newly created classes and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Use ref count as a temporary measure to prevent file from eviction until uploaded to Remote

Signed-off-by: Shreyansh Ray <[email protected]>

* Remove method level locks

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle tmp file deletion

Signed-off-by: Shreyansh Ray <[email protected]>

* Nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle delete and close in Composite Directory, log current state of FileCache and correct it's clear method and modify unit and integration tests as per review comments

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor usages of WRITEABLE_REMOTE_INDEX_SETTING to TIERED_REMOTE_INDEX_SETTING

Signed-off-by: Shreyansh Ray <[email protected]>

* Add tests for FileCachedIndexInput and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add additional IT for feature flag disabled

Signed-off-by: Shreyansh Ray <[email protected]>

* Move setting for Partial Locality type behind Feature Flag, fix bug for ref count via cloneMap in FullFileCachedIndexInput and other review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor test and nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add javadocs for FullFileCachedIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor precommit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>

* Fix Writable Warm test for feature flag disabled condition by changing exception type caught

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>
@rayshrey rayshrey added the v2.16.0 Issues and PRs related to version 2.16.0 label Jul 10, 2024
harshavamsi pushed a commit to harshavamsi/OpenSearch that referenced this pull request Jul 12, 2024
… with FileCache (opensearch-project#12782)

* Composite Directory POC

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor TransferManager interface to RemoteStoreFileTrackerAdapter

Signed-off-by: Shreyansh Ray <[email protected]>

* Implement block level fetch for Composite Directory

Signed-off-by: Shreyansh Ray <[email protected]>

* Removed CACHE state from FileTracker

Signed-off-by: Shreyansh Ray <[email protected]>

* Fixes after latest pull

Signed-off-by: Shreyansh Ray <[email protected]>

* Add new setting for warm, remove store type setting, FileTracker and RemoteStoreFileTrackerAdapter, CompositeDirectoryFactory and update Composite Directory implementation

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify TransferManager - replace BlobContainer with Functional Interface to fetch an InputStream instead

Signed-off-by: Shreyansh Ray <[email protected]>

* Reuse OnDemandBlockSnapshotIndexInput instead of OnDemandBlockCompositeIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify constructors to avoid breaking public api contract and code review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add experimental annotations for newly created classes and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Use ref count as a temporary measure to prevent file from eviction until uploaded to Remote

Signed-off-by: Shreyansh Ray <[email protected]>

* Remove method level locks

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle tmp file deletion

Signed-off-by: Shreyansh Ray <[email protected]>

* Nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle delete and close in Composite Directory, log current state of FileCache and correct it's clear method and modify unit and integration tests as per review comments

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor usages of WRITEABLE_REMOTE_INDEX_SETTING to TIERED_REMOTE_INDEX_SETTING

Signed-off-by: Shreyansh Ray <[email protected]>

* Add tests for FileCachedIndexInput and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add additional IT for feature flag disabled

Signed-off-by: Shreyansh Ray <[email protected]>

* Move setting for Partial Locality type behind Feature Flag, fix bug for ref count via cloneMap in FullFileCachedIndexInput and other review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor test and nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add javadocs for FullFileCachedIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor precommit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>
kkewwei pushed a commit to kkewwei/OpenSearch that referenced this pull request Jul 24, 2024
… integrating it with FileCache (opensearch-project#14489)

* [Writable Warm] Composite Directory implementation and integrating it with FileCache (opensearch-project#12782)

* Composite Directory POC

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor TransferManager interface to RemoteStoreFileTrackerAdapter

Signed-off-by: Shreyansh Ray <[email protected]>

* Implement block level fetch for Composite Directory

Signed-off-by: Shreyansh Ray <[email protected]>

* Removed CACHE state from FileTracker

Signed-off-by: Shreyansh Ray <[email protected]>

* Fixes after latest pull

Signed-off-by: Shreyansh Ray <[email protected]>

* Add new setting for warm, remove store type setting, FileTracker and RemoteStoreFileTrackerAdapter, CompositeDirectoryFactory and update Composite Directory implementation

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify TransferManager - replace BlobContainer with Functional Interface to fetch an InputStream instead

Signed-off-by: Shreyansh Ray <[email protected]>

* Reuse OnDemandBlockSnapshotIndexInput instead of OnDemandBlockCompositeIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify constructors to avoid breaking public api contract and code review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add experimental annotations for newly created classes and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Use ref count as a temporary measure to prevent file from eviction until uploaded to Remote

Signed-off-by: Shreyansh Ray <[email protected]>

* Remove method level locks

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle tmp file deletion

Signed-off-by: Shreyansh Ray <[email protected]>

* Nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle delete and close in Composite Directory, log current state of FileCache and correct it's clear method and modify unit and integration tests as per review comments

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor usages of WRITEABLE_REMOTE_INDEX_SETTING to TIERED_REMOTE_INDEX_SETTING

Signed-off-by: Shreyansh Ray <[email protected]>

* Add tests for FileCachedIndexInput and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add additional IT for feature flag disabled

Signed-off-by: Shreyansh Ray <[email protected]>

* Move setting for Partial Locality type behind Feature Flag, fix bug for ref count via cloneMap in FullFileCachedIndexInput and other review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor test and nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add javadocs for FullFileCachedIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor precommit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>

* Fix Writable Warm test for feature flag disabled condition by changing exception type caught

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>
Signed-off-by: kkewwei <[email protected]>
wdongyu pushed a commit to wdongyu/OpenSearch that referenced this pull request Aug 22, 2024
… with FileCache (opensearch-project#12782)

* Composite Directory POC

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor TransferManager interface to RemoteStoreFileTrackerAdapter

Signed-off-by: Shreyansh Ray <[email protected]>

* Implement block level fetch for Composite Directory

Signed-off-by: Shreyansh Ray <[email protected]>

* Removed CACHE state from FileTracker

Signed-off-by: Shreyansh Ray <[email protected]>

* Fixes after latest pull

Signed-off-by: Shreyansh Ray <[email protected]>

* Add new setting for warm, remove store type setting, FileTracker and RemoteStoreFileTrackerAdapter, CompositeDirectoryFactory and update Composite Directory implementation

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify TransferManager - replace BlobContainer with Functional Interface to fetch an InputStream instead

Signed-off-by: Shreyansh Ray <[email protected]>

* Reuse OnDemandBlockSnapshotIndexInput instead of OnDemandBlockCompositeIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Modify constructors to avoid breaking public api contract and code review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add experimental annotations for newly created classes and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Use ref count as a temporary measure to prevent file from eviction until uploaded to Remote

Signed-off-by: Shreyansh Ray <[email protected]>

* Remove method level locks

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle tmp file deletion

Signed-off-by: Shreyansh Ray <[email protected]>

* Nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Handle delete and close in Composite Directory, log current state of FileCache and correct it's clear method and modify unit and integration tests as per review comments

Signed-off-by: Shreyansh Ray <[email protected]>

* Refactor usages of WRITEABLE_REMOTE_INDEX_SETTING to TIERED_REMOTE_INDEX_SETTING

Signed-off-by: Shreyansh Ray <[email protected]>

* Add tests for FileCachedIndexInput and review comment fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add additional IT for feature flag disabled

Signed-off-by: Shreyansh Ray <[email protected]>

* Move setting for Partial Locality type behind Feature Flag, fix bug for ref count via cloneMap in FullFileCachedIndexInput and other review fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor test and nit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

* Add javadocs for FullFileCachedIndexInput

Signed-off-by: Shreyansh Ray <[email protected]>

* Minor precommit fixes

Signed-off-by: Shreyansh Ray <[email protected]>

---------

Signed-off-by: Shreyansh Ray <[email protected]>
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 v2.16.0 Issues and PRs related to version 2.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.