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

Compatibility with segment replication #989

Closed
Tracked by #8211
dreamer-89 opened this issue Aug 21, 2023 · 8 comments
Closed
Tracked by #8211

Compatibility with segment replication #989

dreamer-89 opened this issue Aug 21, 2023 · 8 comments
Assignees
Labels
bug Something isn't working v2.10.0 Issues targeting release v2.10.0

Comments

@dreamer-89
Copy link
Member

dreamer-89 commented Aug 21, 2023

Coming from #936 (comment) and with core now supporting real time reads for segment replication enabled indices (opensearch-project/OpenSearch#8536), we can revert #948 which enforces DOCUMENT replication strategy.

CC @kaituo @ohltyler

Request owners to label this issue with 2.10.0 release.

@dreamer-89 dreamer-89 added bug Something isn't working untriaged labels Aug 21, 2023
@kaituo
Copy link
Collaborator

kaituo commented Aug 21, 2023

will respond later.

@jackiehanyang
Copy link
Collaborator

revert change pr created - #999

@dreamer-89 do you have any instruction on how to test this to ensure the core side change is working well with AD?

@dreamer-89
Copy link
Member Author

revert change pr created - #999

@dreamer-89 do you have any instruction on how to test this to ensure the core side change is working well with AD?

Thanks @jackiehanyang for working on this change. I commented on revert PR #999

@kaituo
Copy link
Collaborator

kaituo commented Aug 31, 2023

I've conducted a comprehensive review of our backend and would like to share my insights on two key components:

  1. State Index Query:
  • The real-time performance is satisfactory, especially since it primarily supports the profile API. The RAW feature is not required.
  • For historical detectors, a change may or may not be necessary. Interestingly, ml-commons has no reported issues and historical detectors have implementation similar to ml-commons. You can refer to Issue #1023 which Yaliang has reviewed and signed off on. Should any modifications be required, TaskManager.getAndExecuteOnLatestTasks should prioritize primary shards to locate the most recent task.
  1. Result Index Query from Alerting:

As indicated in Issue #974, there's a slight delay of a few seconds. However, given that we operate with a minute-level delay, I believe this won't pose any significant concerns.

In light of the above, I am inclined to approve Pull Request #999 . This should allow us to finalize the seg replication compatibility.

@dreamer-89
Copy link
Member Author

Thanks @kaituo for the summary and explanation. Do we need anything else other than merging PR #999 ? If not, do you mind merging #999 and closing this issue.

@kaituo
Copy link
Collaborator

kaituo commented Sep 1, 2023

We do need another approver.

@dreamer-89
Copy link
Member Author

@kaituo @jackiehanyang : As PR #999 is merged and backported. Do you mind closing this issue if nothing else is pending.

@dreamer-89
Copy link
Member Author

@kaituo @owaiskazi19 @jackiehanyang : Mind closing this issue if nothing else is pending

@kaituo kaituo closed this as completed Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v2.10.0 Issues targeting release v2.10.0
Projects
None yet
Development

No branches or pull requests

4 participants