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

Add a new node role 'search' which is dedicated to provide search capability #4689

Merged
merged 9 commits into from
Oct 11, 2022

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Oct 6, 2022

Description

Add a new node role which is dedicated to provide search capability.
The role name is search.

Issues Resolved

Resolve #4652

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng changed the title Add a new node role - remote searcher Add a new node role - remote_searcher Oct 6, 2022
@tlfeng tlfeng changed the title Add a new node role - remote_searcher Add a new node role which can provide search capability for remote shard Oct 6, 2022
Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request feature New feature or request Indexing & Search v3.0.0 Issues and PRs related to version 3.0.0 and removed feature New feature or request labels Oct 6, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

…archer

Signed-off-by: Tianli Feng <ftianli@amazon.com>

# Conflicts:
#	CHANGELOG.md
@tlfeng tlfeng force-pushed the node-role-remote-searcher branch from d079bd3 to 017b202 Compare October 6, 2022 04:44
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

Tianli Feng added 2 commits October 10, 2022 09:41
Signed-off-by: Tianli Feng <ftianli@amazon.com>
…archer

Signed-off-by: Tianli Feng <ftianli@amazon.com>

# Conflicts:
#	CHANGELOG.md
@tlfeng tlfeng changed the title Add a new node role which can provide search capability for remote shard Add a new node role which is dedicated to provide search capability Oct 10, 2022
@tlfeng tlfeng marked this pull request as ready for review October 10, 2022 16:43
@tlfeng tlfeng requested review from a team and reta as code owners October 10, 2022 16:43
@tlfeng tlfeng changed the title Add a new node role which is dedicated to provide search capability Add a new node role 'search' which is dedicated to provide search capability Oct 10, 2022
Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng added backport 2.x Backport to 2.x branch v2.4.0 'Issues and PRs related to version v2.4.0' labels Oct 10, 2022
if (nodeVersion.onOrAfter(Version.V_2_4_0)) {
return this;
} else {
return DiscoveryNodeRole.DATA_ROLE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we necessarily need a fallback?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me check what will be like when there is no fallback. So far I think without the fallback, the new role will be taken as "UnknownRole" when communicating with node in older versions, but I need to figure out the impact.

static class UnknownRole extends DiscoveryNodeRole {

Copy link
Collaborator Author

@tlfeng tlfeng Oct 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the fallback in the latest commit 9a05a90.
Looks like nothing wrong without the fallback for the new role.

I started 2 nodes to form a cluster.
In 3.0.0 node:

$ curl -XGET "http://localhost:9201/_cat/nodes?v&pretty"
ip        heap.percent ram.percent cpu load_1m load_5m load_15m node.role node.roles  cluster_manager name
127.0.0.1           19          17   0    0.09    0.15     0.09 s         search      -               node-3.0
127.0.0.1           61          17   0    0.09    0.15     0.09 dm        data,master *               node-2.4

In 2.4.0 node:

$ curl -XGET "http://localhost:9200/_cat/nodes?v&pretty"   
ip        heap.percent ram.percent cpu load_1m load_5m load_15m node.role node.roles  cluster_manager name
127.0.0.1           18          17   0    0.04    0.15     0.08 -         search      -               node-3.0
127.0.0.1           60          17   0    0.04    0.15     0.08 dm        data,master *               node-2.4

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Tianli Feng added 2 commits October 10, 2022 21:31
…archer

Signed-off-by: Tianli Feng <ftianli@amazon.com>
Signed-off-by: Tianli Feng <ftianli@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #4689 (9a05a90) into main (9f415eb) will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #4689      +/-   ##
============================================
+ Coverage     70.59%   70.81%   +0.21%     
- Complexity    57514    57582      +68     
============================================
  Files          4660     4660              
  Lines        276466   276470       +4     
  Branches      40285    40285              
============================================
+ Hits         195170   195780     +610     
+ Misses        65060    64421     -639     
- Partials      16236    16269      +33     
Impacted Files Coverage Δ
...rest/src/main/java/org/opensearch/client/Node.java 92.72% <100.00%> (+1.98%) ⬆️
...ava/org/opensearch/cluster/node/DiscoveryNode.java 86.15% <100.00%> (+1.10%) ⬆️
...org/opensearch/cluster/node/DiscoveryNodeRole.java 88.05% <100.00%> (+1.90%) ⬆️
...pensearch/client/cluster/RemoteConnectionInfo.java 0.00% <0.00%> (-68.30%) ⬇️
...a/org/opensearch/client/cluster/ProxyModeInfo.java 0.00% <0.00%> (-60.00%) ⬇️
...cluster/coordination/PendingClusterStateStats.java 20.00% <0.00%> (-48.00%) ⬇️
...r/src/main/java/org/opensearch/http/HttpStats.java 21.05% <0.00%> (-47.37%) ⬇️
...search/search/aggregations/pipeline/EwmaModel.java 24.44% <0.00%> (-44.45%) ⬇️
.../opensearch/client/cluster/RemoteInfoResponse.java 61.53% <0.00%> (-38.47%) ⬇️
...pensearch/action/ingest/DeletePipelineRequest.java 31.25% <0.00%> (-37.50%) ⬇️
... and 473 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@kotwanikunal kotwanikunal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tlfeng tlfeng merged commit c1272c1 into opensearch-project:main Oct 11, 2022
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-4689-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c1272c181e6bd96d1db725dfbbcd5ac8e6058502
# Push it to GitHub
git push --set-upstream origin backport/backport-4689-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4689-to-2.x.

@tlfeng tlfeng deleted the node-role-remote-searcher branch October 11, 2022 17:21
tlfeng pushed a commit to tlfeng/OpenSearch that referenced this pull request Oct 11, 2022
…ability (opensearch-project#4689)

Signed-off-by: Tianli Feng <ftianli@amazon.com>
dblock pushed a commit that referenced this pull request Oct 11, 2022
…ability (#4689) (#4739)

Signed-off-by: Tianli Feng <ftianli@amazon.com>

Signed-off-by: Tianli Feng <ftianli@amazon.com>
ashking94 pushed a commit to ashking94/OpenSearch that referenced this pull request Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch enhancement Enhancement or improvement to existing feature or request Indexing & Search v2.4.0 'Issues and PRs related to version v2.4.0' v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Searchable Snapshots] Add a new node role for remote search capabilities
4 participants