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

[Backport 2.0] [BUG] fix healthcheck logic to expect object and return ids #2298

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Sep 8, 2022

Backport 3496526 from #2277

…ch-project#2277)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla kavilla requested a review from a team as a code owner September 8, 2022 19:33
@kavilla
Copy link
Member Author

kavilla commented Sep 8, 2022

Expected failure because there isn't a patch version of OpenSearch built for this version of OpenSearch Dashboards.

@kavilla kavilla merged commit 52188a0 into opensearch-project:2.0 Sep 8, 2022
pjfitzgibbons pushed a commit to pjfitzgibbons/OpenSearch-Dashboards that referenced this pull request Oct 24, 2022
…ch-project#2277) (opensearch-project#2298)

Original implementation incorrectly assumed the return list of
nodes was an object array.
This PR:
opensearch-project#2232

Addressed the return but didn't catch the nodes.find in the return
which is a function for an array.

Also, refactors to return a list of node_ids because the original
implementation indicated that it should but it can return node ids
but it never did. It only returned `null` or `_local`, the problem
with this approach is that it doesn't expect valid node version
with different DIs or filter out nodes when we pass `null` as the node
ID for the node info call because it was fan out the request to all
nodes.

Now this function will return `_local` if all the nodes share the
same cluster_id using a greedy approach since we can assume it is
all the same version.

Will return node ids, if the cluster_id are different so it will
pass a CSV to the node info call and return the info for those nodes.
And null if no cluster_id is present, ie, fan out the response.

Original issue:
opensearch-project#2203

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
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.

3 participants