[Backport 1.x] [BUG] fix healthcheck logic to expect object and return ids #2300
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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>
Flyingliuhub
approved these changes
Sep 8, 2022
ananzh
approved these changes
Sep 8, 2022
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Sep 8, 2022
…2300) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34)
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Sep 8, 2022
…2300) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34)
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Sep 8, 2022
…2300) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34)
This was referenced Sep 8, 2022
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Sep 8, 2022
…2300) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34)
kavilla
added a commit
that referenced
this pull request
Sep 8, 2022
…2300) (#2304) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34) Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
kavilla
added a commit
that referenced
this pull request
Sep 8, 2022
…2300) (#2302) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34) Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
kavilla
added a commit
that referenced
this pull request
Sep 9, 2022
…2300) (#2301) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34) Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
Flyingliuhub
added a commit
that referenced
this pull request
Sep 9, 2022
* [BUG] fix healthcheck logic to expect object and return ids (#2277) (#2300) (#2301) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34) Co-authored-by: Kawika Avilla <kavilla414@gmail.com> * change version Signed-off-by: Tao liu <liutaoaz@amazon.com> Signed-off-by: Tao liu <liutaoaz@amazon.com> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
Flyingliuhub
pushed a commit
that referenced
this pull request
Sep 12, 2022
…2300) (#2303) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: #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: #2203 Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> (cherry picked from commit e10ba34) Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 3496526 from #2277