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

[Segment Replication] Test all plugin compatibility #6602

Closed
mch2 opened this issue Mar 9, 2023 · 6 comments
Closed

[Segment Replication] Test all plugin compatibility #6602

mch2 opened this issue Mar 9, 2023 · 6 comments
Assignees
Labels
distributed framework enhancement Enhancement or improvement to existing feature or request

Comments

@mch2
Copy link
Member

mch2 commented Mar 9, 2023

Leading up to GA of segment replication, we need to do a round of sanity testing with plugins.

This issue is to track that effort & results.

To do this testing we can run each repo's integTests that are provided as part of CI builds, but the tests will need to build a separate tarball with segrep enabled by default.

General steps:

  1. Clone build repo
  2. Update input manifest example: https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.6.0/opensearch-2.6.0.yml
  3. Fix the refs in manifest to point to a custom segrep enabled tar.
  4. Run ./build.sh
  5. Should give all plugin artifacts in the local
  6. Create test-manifest Example: https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.6.0/opensearch-2.6.0-test.yml that uses multi node. example: https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.7.0/opensearch-2.7.0-test.yml#L11-L17
  7. run ./test.sh integ-test <manifest> --component ml-commons --paths opensearch=<locally generated tar folder>
@Rishikesh1159
Copy link
Member

To test all plugins compatibility, we first created an issue to add a cluster setting to enable segment replication as default replication strategy. This PR: #6791 resolved the issue and added a new cluster setting.

After adding new cluster setting next step is to create a test-manifest with segment replication enabled. So that we can run two opensearch build integ-test runs, one with segment replication enabled on 2.x branch(new segrep test-manifest) and another run segment replication disabled on 2.x branch(normal 2.7 test-manifest).

Created this Issue on Opensearch-build repo to add a new test manifest for segment replication. With this PR:opensearch-project/opensearch-build#3345 merged in, we can now trigger builds with segment replication.

Now we have:
Opensearch build integ-test run(on 2.x branch) with Segment replication enabled: https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test/detail/integ-test/3979/pipeline/
Opensearch build integ-test run(on 2.x branch) with Segment Replication disabled: https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test/detail/integ-test/3983/pipeline/

Results:
-> From above runs we see that KNN plugin and CCR plugins fails in these.
-> We can ignore KNN plugin as integ-tests were passing without security and reason for failing with security is not related to segment replication.

2023-03-24 22:45:39 ERROR    | k-NN                 | with-security        | FAIL  |
2023-03-24 22:45:39 INFO     | k-NN                 | without-security     | PASS  |

-> But CCR plugin is having real integ-tests failures with segment replication.

2023-03-24 23:20:35 ERROR    | cross-cluster-replication | with-security        | FAIL  |
2023-03-24 23:20:35 ERROR    | cross-cluster-replication | without-security     | FAIL  |

-> We need to create new issue to resolve these integ-test failures with segment replication

@anasalkouz
Copy link
Member

@ankitkala any thoughts on the above failure in CCR plugin?

@ankitkala
Copy link
Member

ankitkala commented Mar 28, 2023

We did verify CCR plugin tests with SegRep enabled on all indices here. We had to force docrep on ccr metadata index(link) for tests to pass.

@Rishikesh1159 Can you try with the change mentioned above?

@ankitkala
Copy link
Member

Alternatively, I think you can force docrep for hidden indices as well(i.e. starting with .) here

@Rishikesh1159
Copy link
Member

Alternatively, I think you can force docrep for hidden indices as well(i.e. starting with .) here

I think we don't have a good way to filter out which indices are CCR's system/hidden indices. There can be other indices which are not CCR's hidden index and are starting with .

@Rishikesh1159
Copy link
Member

Rishikesh1159 commented Mar 30, 2023

Thanks @ankitkala I tried what you suggested in comment. And I don't see any compatibility issues with segrep and ccr as you previously mentioned, all integtests are passing. But if we are using hidden/system indices of CCR with segrep some tests are failing, as we previously saw. So, we have to use docrep for ccr's hidden/system indices.

We recently added a new condition in opensearch that checks if an index is system index or not, if it is a system index we force it to use document replication.

But problem is, CCR's hidden/system indices are not recognized by opensearch as a system index here. I think the concept of system index we have in opensearch here in SystemIndices, SystemIndexDescriptor and plugins like CCR is different.

So, for now we don't have a good way to make changes on opensearch in segrep as default to work with CCR. One solution here will be, we can make a change in CCR repo to strictly use docrep for all the system/hidden indices.

But for long term solutions we have two options:
-> Make segment replication work with system/hidden indices of CCR and other plugins.
-> Other option would be is to make all opensearch plugin's system/hidden indices compatible with or similar to system index we have in opensearch here in SystemIndices. It means all plugins have to on-board with new/ opensearch way of system index.

For now with segment replication GA release in 2.7 version, will be go ahead with solution of making a change in CCR repo to strictly use docrep for all the system/hidden indices.

Closing this issue as we have a solution for now.

This was referenced Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed framework enhancement Enhancement or improvement to existing feature or request
Projects
Status: Done
Development

No branches or pull requests

5 participants