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

Quick updates to the RFS design doc, assuming a later pass #1127

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

chelma
Copy link
Member

@chelma chelma commented Nov 11, 2024

Description

Performed a quick update to the RFS Design doc to clear out most of the outdated stuff. I assumed a follow-up pass would be made by @mikaylathompson in her update to he work coordination behavior (see: https://opensearch.atlassian.net/browse/MIGRATIONS-2126).

Issues Resolved

Testing

N/A

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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: Chris Helma <chelma+github@amazon.com>
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.63%. Comparing base (a9badc5) to head (71d9696).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #1127   +/-   ##
=========================================
  Coverage     80.63%   80.63%           
  Complexity     2910     2910           
=========================================
  Files           399      399           
  Lines         14829    14829           
  Branches       1007     1007           
=========================================
  Hits          11958    11958           
  Misses         2260     2260           
  Partials        611      611           
Flag Coverage Δ
gradle-test 78.66% <ø> (ø)
python-test 89.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Collaborator

@gregschohn gregschohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments/typos, but LGTM

@@ -77,20 +76,17 @@ Below is an example for the structure of an Elasticsearch 7.10 snapshot, along w
8. **Snapshot Metadata File**: SMILE encoded; contains things like whether the snapshot succeeded, the number of shards, how many shards succeeded, the ES/OS version, the indices in the snapshot, etc

## Ultra-High Level Design
The responsibility of performing an RFS operation is split into two groups of actors - the RFS Workers, and RFS Scaler (see Figure 1, below).
The responsibility of performing an RFS operation is performed by a group of one or more the RFS Workers (see Figure 1, below).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...one or more the RFS...


### RFS Worker threads

The RFS Worker’s running process has at least two running threads:

* Main Thread - performs the work of moving the data/metadata from the source cluster to the target cluster; starts the Metrics and Healthcheck Threads
* Main Thread - performs the work of moving the data from the source cluster to the target cluster; starts the Metrics and Healthcheck Threads
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There likely is a thread for metrics, but do you care to mention it? There's also more than 1 thread responsible for moving data to the target now that we're using more work fanouts from reactor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just missed that reference to the Metrics thread; thanks for catching.

IIRC from convos w/ @mikaylathompson, we're going to be move from multiple threads for reindexing to a single thread in order for the checkpointing to work as intended. We'll just need to make sure that we update the doc here for the final behavior.

* (A10) - Elasticsearch Shards can only be migrated after their Elasticsearch Index has been migrated
* (A11) - Elasticsearch Shards can be migrated in parallel without ordering concerns
* (A3) - Re-doing portions of the overall RFS operation is fine, as long as every portion is completed at least once
* (A4) - Elasticsearch Shards can only be migrated after their Elasticsearch Index has been migrated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the discussions about defaults and manually setting metadata, this could also be "migrated or created"

chelma and others added 2 commits November 12, 2024 04:43
Signed-off-by: Chris Helma <chelma+github@amazon.com>
@chelma chelma merged commit 41bbfc4 into opensearch-project:main Nov 12, 2024
16 checks passed
@chelma chelma deleted the rfs-readme branch November 12, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants