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

Search stats for coordinator node misses total search request time #10768

Closed
jainankitk opened this issue Oct 20, 2023 · 11 comments · Fixed by #15054
Closed

Search stats for coordinator node misses total search request time #10768

jainankitk opened this issue Oct 20, 2023 · 11 comments · Fixed by #15054
Assignees
Labels
backport 2.x Backport to 2.x branch enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers Search Search query, autocomplete ...etc

Comments

@jainankitk
Copy link
Collaborator

Currently, the coordinator node only publishes the phase breakdown metrics. It should also publish the total search request time as part of node stats on the coordinator node

Related #8381, #8386

@jainankitk jainankitk added enhancement Enhancement or improvement to existing feature or request untriaged Search Search query, autocomplete ...etc and removed untriaged labels Oct 20, 2023
@jainankitk
Copy link
Collaborator Author

@dzane17 - Are you able to take stab at this one?

@msfroh
Copy link
Collaborator

msfroh commented Oct 20, 2023

Is this likely to be different from the took time? If so, how?

@msfroh
Copy link
Collaborator

msfroh commented Oct 20, 2023

Is this likely to be different from the took time? If so, how?

Oh... This is for the stats as opposed to the debugging info reported in SearchResponse?

@jainankitk
Copy link
Collaborator Author

Oh... This is for the stats as opposed to the debugging info reported in SearchResponse?

Yeah, I missed that in the initial PR!

@jainankitk jainankitk assigned jainankitk and unassigned jainankitk Oct 25, 2023
@dzane17
Copy link
Contributor

dzane17 commented Oct 25, 2023

Yes, I will take care of it

@jainankitk
Copy link
Collaborator Author

@dzane17 - Have we already addressed this issue?

@kkhatua kkhatua added the good first issue Good for newcomers label Jun 3, 2024
@sandeshkr419
Copy link
Contributor

@dzane17 Are you still working on this?

@dzane17
Copy link
Contributor

dzane17 commented Jun 20, 2024

No, the team has decided to deprioritize this for now. I will unassign myself.

@dzane17 dzane17 removed their assignment Jun 20, 2024
@jainankitk
Copy link
Collaborator Author

As per offline discussion, @dzane17 should be able to prioritize this for 2.17. Reassigning

@jainankitk jainankitk assigned dzane17 and unassigned jainankitk Jul 24, 2024
@jainankitk jainankitk added the backport 2.x Backport to 2.x branch label Jul 24, 2024
@dzane17
Copy link
Contributor

dzane17 commented Jul 26, 2024

@jainankitk Can you confirm what the final output should look like. I have added the took section below as a proposal. I think it should be within the "request" object since this is where we keep all coordinator-level metrics. But the name should distinguish it from other metrics within "request" which are per phase.

...
      "indices" : {
        "search" : {
          "open_contexts" : 0,
          "query_total" : 4,
          "query_time_in_millis" : 22,
          "query_current" : 0,
          "concurrent_query_total" : 0,
          "concurrent_query_time_in_millis" : 0,
          "concurrent_query_current" : 0,
          "concurrent_avg_slice_count" : 0.0,
          "fetch_total" : 4,
          "fetch_time_in_millis" : 3,
          "fetch_current" : 0,
          "scroll_total" : 0,
          "scroll_time_in_millis" : 0,
          "scroll_current" : 0,
          "point_in_time_total" : 0,
          "point_in_time_time_in_millis" : 0,
          "point_in_time_current" : 0,
          "suggest_total" : 0,
          "suggest_time_in_millis" : 0,
          "suggest_current" : 0,
          "search_idle_reactivate_count_total" : 0,
          "request" : {
            "took" : {
              "time_in_millis" : 71,
              "current" : 0,
              "total" : 2
            },
            "dfs_pre_query" : {
              "time_in_millis" : 0,
              "current" : 0,
              "total" : 0
            },
            "query" : {
              "time_in_millis" : 68,
              "current" : 0,
              "total" : 2
            },
            "fetch" : {
              "time_in_millis" : 3,
              "current" : 0,
              "total" : 2
            },
            "dfs_query" : {
              "time_in_millis" : 0,
              "current" : 0,
              "total" : 0
            },
            "expand" : {
              "time_in_millis" : 0,
              "current" : 0,
              "total" : 2
            },
            "can_match" : {
              "time_in_millis" : 0,
              "current" : 0,
              "total" : 0
            }
          }
        }

@jainankitk
Copy link
Collaborator Author

took sounds correct to me since it correlates directly with the client search request took time

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 good first issue Good for newcomers Search Search query, autocomplete ...etc
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants