Skip to content

Add wikipedia track ESQL profiling#984

Merged
carlosdelest merged 33 commits intoelastic:masterfrom
carlosdelest:enhancement/so_vector-esql-wikipedia-profiling
Jan 9, 2026
Merged

Add wikipedia track ESQL profiling#984
carlosdelest merged 33 commits intoelastic:masterfrom
carlosdelest:enhancement/so_vector-esql-wikipedia-profiling

Conversation

@carlosdelest
Copy link
Member

Adds ES|QL profiling metrics to the ES|QL queries in the wikipedia track.

Related #973, elastic/elasticsearch#139540, elastic/elasticsearch#138564

@carlosdelest carlosdelest marked this pull request as ready for review January 9, 2026 09:06

# Build took_ms entries for each profiled phase
result = {}
if profile:
Copy link

Choose a reason for hiding this comment

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

we have a check here to see that profile is present, but we don't have one after this if statement when we iterate over the drivers and plans.
I wonder if profile can ever be empty - and if it can - wouldn't it be better to just return early, e.g:

result = {}
if not profile:
  return result

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense - I modified both the wikipedia and so_vector versions in 92e06d5

@carlosdelest carlosdelest requested a review from ioanatia January 9, 2026 11:20
Copy link

@ioanatia ioanatia left a comment

Choose a reason for hiding this comment

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

i wonder if we can avoid the duplication of EsqlProfileRunner from both so_vector and the wikipedia track since they look identical.
but fine with me to follow up on that separately.
We could for now add a TODO comment in the declarations of EsqlProfileRunner to deduplicate them. This can also act as a reminder that if someone modifies just one of them, it might be useful to do the same for the other.

@carlosdelest
Copy link
Member Author

i wonder if we can avoid the duplication of EsqlProfileRunner from both so_vector and the wikipedia track since they look identical.

Yep - the only way that I found is to move them to rally itself as a new runner.

Given that this is still WIP as we may add / change the profile information, I opted to duplicate the code for now (this is similar to KnnRecallRunner in so_vector, dense_vector, msmarco and openai ).

+1 to move that to rally eventually.

@carlosdelest carlosdelest enabled auto-merge (squash) January 9, 2026 11:36
@carlosdelest carlosdelest merged commit 7dc85dc into elastic:master Jan 9, 2026
15 checks passed
@esbenchmachine esbenchmachine added the backport pending Awaiting backport to stable release branch label Jan 9, 2026
@esbenchmachine
Copy link
Collaborator

@carlosdelest
A backport is pending for this PR. Please add all required vX.Y version labels.

  • If it is intended for the current Elasticsearch release version, apply the corresponding version label.
  • If it also supports past released versions, add those labels too.
  • If it only targets a future version, wait until that version label exists and then add it.
    (Each rally-tracks version label is created during the feature freeze of a new Elasticsearch branch).

Backporting entails:

  1. Ensure the correct version labels exist in this PR.
  2. Ensure backport PRs have backport label and are passing tests.
  3. Merge backport PRs (you can approve yourself and enable auto-merge).
  4. Remove backport pending label from this PR once all backport PRs are merged.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport pending Awaiting backport to stable release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants