-
Notifications
You must be signed in to change notification settings - Fork 40
Cleanup RFS source and target versions, add support for RFS 5.x, 7.x target with same-version migrations #1373
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
Cleanup RFS source and target versions, add support for RFS 5.x, 7.x target with same-version migrations #1373
Conversation
02e04dc
to
0fa5a7a
Compare
0fa5a7a
to
1d2377d
Compare
…ations Signed-off-by: Andre Kurait <[email protected]>
1d2377d
to
e50862e
Compare
Signed-off-by: Andre Kurait <[email protected]>
Signed-off-by: Andre Kurait <[email protected]>
…y across versions Signed-off-by: Andre Kurait <[email protected]>
64cd4b1
to
085ef1e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1373 +/- ##
============================================
- Coverage 80.12% 0 -80.13%
============================================
Files 440 0 -440
Lines 16184 0 -16184
Branches 1078 0 -1078
============================================
- Hits 12968 0 -12968
+ Misses 2582 0 -2582
+ Partials 634 0 -634
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BASE(Map.of("discovery.type", "single-node", | ||
"path.repo", CLUSTER_SNAPSHOT_DIR, | ||
"ES_JAVA_OPTS", "-Xms512m -Xmx512m")), | ||
"ES_JAVA_OPTS", "-Xms2g -Xmx2g", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had more consistent ES performance with this larger memory and settings below. We should be running all containers in isolatedTests so less concern of contention with parallelization
114adf0
to
adae298
Compare
adae298
to
9adf692
Compare
Wait, do we support/want to support 5.x as a target? Why? |
DataGenerator/src/test/java/org/opensearch/migrations/DataGeneratorEndToEndTest.java
Outdated
Show resolved
Hide resolved
...mSnapshotMigration/src/test/java/org/opensearch/migrations/bulkload/LeaseExpirationTest.java
Outdated
Show resolved
Hide resolved
...tsFromSnapshotMigration/src/test/java/org/opensearch/migrations/bulkload/SourceTestBase.java
Outdated
Show resolved
Hide resolved
MetadataMigration/src/test/java/org/opensearch/migrations/CustomTransformationTest.java
Outdated
Show resolved
Hide resolved
...java/org/opensearch/migrations/bulkload/version_es_6_8/OpenSearchWorkCoordinator_ES_6_8.java
Outdated
Show resolved
Hide resolved
...main/java/org/opensearch/migrations/bulkload/workcoordination/OpenSearchWorkCoordinator.java
Show resolved
Hide resolved
...t/java/org/opensearch/migrations/bulkload/workcoordination/OpenSearchWorkCoodinatorTest.java
Show resolved
Hide resolved
...ensearch/migrations/bulkload/workcoordination/WorkCoordinatorErrantAcquisitonsRetryTest.java
Show resolved
Hide resolved
9adf692
to
61e693f
Compare
This is useful to us in an rfs-only support which will allow us to use RFS to generate large snapshots (e.g. 10 TB) |
61e693f
to
a7f3a41
Compare
a7f3a41
to
4d87ee5
Compare
4d87ee5
to
0a342b4
Compare
0a342b4
to
9af9f68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change the runtime goes up to 57m - that seems like a huge jump can we see about reducing that?
gradle-tests (:DocumentsFromSnapshotMigration:jacocoTestReport)
succeeded 3 days ago in 57m 15s
...rc/main/java/org/opensearch/migrations/bulkload/workcoordination/WorkCoordinatorFactory.java
Show resolved
Hide resolved
RFS/src/test/java/org/opensearch/migrations/bulkload/workcoordination/WorkCoordinatorTest.java
Outdated
Show resolved
Hide resolved
Thanks for creating that draft PR [1] for improving the test stripping - that would reduce my concerns about the test runtime |
9af9f68
to
cde7841
Compare
Signed-off-by: Andre Kurait <[email protected]>
cde7841
to
0be43e3
Compare
@AndreKurait Would you mind holding off on merging this change until we've [1] has been merged? |
Addressed feedback with #1376 merged into main
Related fix for supporting ES 5 as a target: https://opensearch.atlassian.net/browse/MIGRATIONS-2464. |
Description
Add rfs-only for 5x->5x, 7.10->7.10 migrations with tests.
Cleanup how work coordination is done, use
refresh=wait_for
over individual_refresh
callsIssues Resolved
Testing
Unit tests
Check List
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.