-
Notifications
You must be signed in to change notification settings - Fork 90
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
remote indices support #799
base: main
Are you sure you want to change the base?
remote indices support #799
Conversation
hi @lezzago @AWSHurneyt , could you please review this PR when you find a moment? These changes would be very helpful for adding multi cluster capabilities. |
d5abeac
to
98d8110
Compare
@plin13 We're actually actively working on enhancing the alerting plugin to support cross-cluster monitors, and are aiming for a 2.12 or 2.13 release. We have some additional requirements for the initial release of the enhancement that involve refactoring both from back and frontend plugins; so we'd like to hold off on merging in your changes. However, we would be very interested in scheduling some time to discuss your use cases to make sure they're covered by what we're planning to support. @dtaivpp will reach out to set up some time. |
@AWSHurneyt thanks for your feedback. Do you know what changes are being aimed for 2.12/2.13? We have reviewed #796 and i believe some of those requirements can be addressed with this PR. As such, can this PR be reviewed and accepted and then build the additional requirements on top it? I can help implement if needed. I have created another feature request to add support for backend roles in the frontend #860. We would like to include this as well in 2.12/2.13 and i will be submitting a PR for it in the next couple of weeks. @dtaivpp please let us know when we can have a call to discuss the next steps. The earlier the better for us (first or second week of Jan). cc: @plin13 |
For visibility, we discussed over a call last week that we'd like to get these changes in for the 2.12 release. The internal team is working on more extensive enhancements for this feature, but those changes likely won't be available until v2.13. Having the changes in this PR merged will offer some immediate use for query and bucket monitors; and the internal team's enhancements will build upon these changes to improve the UI experience, and support cluster metrics monitors. Document monitors will be supported in a subsequent release. @smuthukaruppannp Would you be able to add some tests to this PR, or enhance existing tests to help improve confidence in these changes? |
@AWSHurneyt, thanks for the feedback. I am working on adding new test cases for verifying the new functionality. For integration tests, we would need two or more clusters running. I am looking into how we can achieve that. One option would be to make two clusters optional and run remote cluster test cases only when two are more clusters are configured in Should i add a check to ensure that remote indices are only supported for query/bucket monitors and disabled for cluster-metrics/document monitors? |
@smuthukaruppannp For transparency, we're working very hard to get the internal implementation of cross cluster monitors included as an experimental feature in the 2.12 release. In which case, the changes in this PR would no longer be needed since the internal implementation covers these cases. However, if we're not able to get the internal implementation included; we'd like to have this PR available for inclusion. |
Signed-off-by: Seth Muthukaruppan <seth.muthukaruppan@netapp.com>
Signed-off-by: Seth Muthukaruppan <seth.muthukaruppan@netapp.com>
3663a01
to
6a23742
Compare
@AWSHurneyt i have added additional unit and integration test cases as suggested. Please review and let me know (looks like some internal changes have been committed to main) |
Description
During monitor creation, provide options to select remote indices when cross cluster search is involved
<remote-cluster>:<remote-indices-*>
*:<remote-index>
<local-index>,<remote-cluster>:<remote-index>
Issues Resolved
[ENHANCEMENT] Can not create monitor in the coordinating cluster when it involves remote indices #570
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.