Can match phase shard APM metric with search request context #137196
Conversation
- es.search.shards.phases.can_match.duration.histogram
elasticsearchmachine
added
v9.3.0
needs:triage
chrisparrinello
changed the title
chrisparrinello
added
>enhancement
Team:Search Foundations
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
|
Hi @chrisparrinello, I've created a changelog YAML for you. |
server/src/main/java/org/elasticsearch/index/search/stats/ShardSearchPhaseAPMMetrics.java
Outdated
Show resolved
Hide resolved
| responses.add(new CanMatchNodeResponse.ResponseOrFailure(canMatch(shardSearchRequest))); | ||
| indexShard.getSearchOperationListener().onCanMatchPhase(System.nanoTime() - shardCanMatchStartTimeInNanos); | ||
| indexShard.getSearchOperationListener() | ||
| .onCanMatchPhase(shardSearchRequest, System.nanoTime() - shardCanMatchStartTimeInNanos); |
There was a problem hiding this comment.
I dove deeper at the overhead and I am thinking that we are wasting time reanalyzing the same search request over and over when many shards are allocated to the same data node.
Even if we create a new shard search request every time for the purpose of calling can match, the source is always the same, and so are the other arguments that are needed to extract the attributes. I would do that analysis only once for all the shards if possible.
There was a problem hiding this comment.
That makes sense to me. I'll update.
| canMatchPhaseMetric, | ||
| tookInNanos, | ||
| shardSearchRequest, | ||
| null // we don't have the time range filter info at this point in the search |
There was a problem hiding this comment.
I think that getting the time range may be ok to do actually. We already collect it, but we just don't propagate it. I looked deeper at this and came up with #137314 . What do you think?
There was a problem hiding this comment.
I looked at the other PR and I think taking advantage of that would require moving where we're recording this (the CanMatchContext hasn't been created yet) but I think based on our conversation on Wednesday about the coordinator level phase, we should be doing this anyway to cover all of the calls to canMatch (in the can-match phase, the query phase, etc.).
3 participants
Add search request attributes context to: