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

Knife search issue: when search entries are more than 10K #3951

Merged
merged 1 commit into from
Dec 24, 2024

Conversation

talktovikas
Copy link
Collaborator

@talktovikas talktovikas commented Dec 2, 2024

Description

[Please describe what this change achieves]
This PR contains the changes for issue on Jira:
https://progresssoftware.atlassian.net/browse/CHEF-5783

Issues Resolved

When user do
Knife search wildcard [ knife search 'name:*' -i | wc -l] was only providing 10k results where as user gets more than 10 when doing lets say [knife node list | wc -l].
The fix here contains adding track_total_hits value in the opensearch request body, If set as true will give all the data when searching using wildcard approach.

curl -XPUT "http://127.0.0.1:10144/chef/_settings" -d '{ "index" :
{ "max_result_window" : 50000 } 
}' -H "Content-Type: application/json"

which tells the max_window_size a response can accommodate.

To use this feature change the value of

track_total_hits =true

in the erchef which is false by default.
[List any existing issues this PR resolves, or any Discourse or
StackOverflow discussions that are relevant]

Check List

@talktovikas talktovikas requested review from a team as code owners December 2, 2024 13:04
Copy link

netlify bot commented Dec 2, 2024

👷 Deploy Preview for chef-server processing.

Name Link
🔨 Latest commit 0a22296
🔍 Latest deploy log https://app.netlify.com/sites/chef-server/deploys/676976baac8b96000889a6ee

Copy link
Member

@marcparadise marcparadise left a comment

Choose a reason for hiding this comment

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

The changes look fine. Once tests are passing could you squash commits and include a descriptive commit message - it should reference the ticket, as well as a 'why' explanation for the change itself.

@talktovikas talktovikas changed the title Knife issue: when Opensearch has limitation of page size. WIP |Knife issue: when Opensearch has limitation of page size. Dec 3, 2024
@talktovikas talktovikas changed the title WIP |Knife issue: when Opensearch has limitation of page size. WIP | Knife issue: when Opensearch has limitation of page size. Dec 3, 2024
@jashaik jashaik changed the title WIP | Knife issue: when Opensearch has limitation of page size. Knife issue: when Opensearch has limitation of page size. Dec 9, 2024
@@ -194,6 +194,7 @@
{timeout, <%= @solr_timeout %>},
{init_count, <%= @solr_http_init_count %>},
{max_count, <%= @solr_http_max_count %>},
{track_total_hits,true},
Copy link
Contributor

Choose a reason for hiding this comment

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

read the config value from default.rb from attributes(Omnibus).

Copy link
Contributor

Choose a reason for hiding this comment

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

Default value should be false for track_total_hits

@@ -30,6 +30,7 @@ from_params(Provider, ObjType, QueryString, Start, Rows) ->
search_provider = Provider,
start = decode({nonneg_int, "start"}, Start, 0),
rows = decode({nonneg_int, "rows"}, Rows, 1000),
track_total_hits = envy:get(chef_index, track_total_hits, true, boolean),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
track_total_hits = envy:get(chef_index, track_total_hits, true, boolean),
track_total_hits = envy:get(chef_index, track_total_hits, false, boolean),

@talktovikas talktovikas requested a review from a team as a code owner December 12, 2024 08:19
@talktovikas talktovikas force-pushed the vikas/knife_index branch 5 times, most recently from b268a4a to 6ca1f97 Compare December 12, 2024 11:42
@talktovikas talktovikas removed the request for review from a team December 12, 2024 11:45
@talktovikas
Copy link
Collaborator Author

talktovikas commented Dec 12, 2024

@talktovikas talktovikas changed the title Knife issue: when Opensearch has limitation of page size. Knife search issue: when search entries are more than 10K Dec 14, 2024
@talktovikas
Copy link
Collaborator Author

@talktovikas
Copy link
Collaborator Author

Video Demo for this: knife_search_issue.mov

@talktovikas talktovikas force-pushed the vikas/knife_index branch 2 times, most recently from b5fb55b to 624fc6f Compare December 18, 2024 11:36
@github-actions github-actions bot added the Documentation Pulls PR onto docs board so they know it exists label Dec 19, 2024
@talktovikas
Copy link
Collaborator Author

Signed-off-by: talktovikas <[email protected]>

fixing omnibus issue.

Signed-off-by: talktovikas <[email protected]>

fixing pedant issue.

Signed-off-by: talktovikas <[email protected]>

binding at runtime.

Signed-off-by: talktovikas <[email protected]>

Doc Changes for knife index.

Signed-off-by: talktovikas <[email protected]>

adding default to toml file.

Signed-off-by: talktovikas <[email protected]>

Quality Gate failed Quality Gate failed

Failed conditions
2 New issues

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@talktovikas
Copy link
Collaborator Author

@jashaik jashaik merged commit 1f0e618 into main Dec 24, 2024
14 of 16 checks passed
@jashaik jashaik deleted the vikas/knife_index branch December 24, 2024 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Pulls PR onto docs board so they know it exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants