Add Fetch Phase to Search Profile #18664

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.

Sign up for GitHub

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

Jump to bottom

Merged

jainankitk merged 97 commits into opensearch-project:main from andrevandeven:time-fetch

Jul 31, 2025

Contributor

andrevandeven commented Jul 1, 2025 •

edited

Loading

The design will mimic that of query profiling but with a variation in the profile tree. The fetch profile tree will always contain a single root node that contains the following information:

Time spent creating the stored fields visitor
Time spent building fetch sub-phase processors
Time spent switching to the next segment
Time spent loading stored fields for a hit
Time spent loading the document _source

The tree will also contain a child node for each fetch sub-phase that runs (see appendix for a list of all sub-phases). The child node breakdown will contain the following information:

Time spent switching to the next segment
Time spent executing a fetch sub-phase

Note: the above diagram only shows sub-phases for simplicity.

Related Issues

Resolves #1764

Check List

Functionality includes testing.
API changes companion pull request created, if applicable.
Public documentation issue/PR created, if applicable.

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.

github-actions bot added distributed framework enhancement lucene labels

andrevandeven force-pushed the time-fetch branch from 63da118 to 30e991d Compare

July 1, 2025 22:36

Contributor

github-actions bot commented Jul 1, 2025

❌ Gradle check result for 30e991d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

msfroh commented Jul 1, 2025

Ooh, neat! I'll review this week, but in case I forget, please feel free to @ mention me, @andrevandeven.

Contributor

github-actions bot commented Jul 2, 2025

❌ Gradle check result for 82d47ef: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

opensearch-ci-bot mentioned this pull request

[AUTOCUT] Gradle Check Flaky Test Report for VerifyVersionConstantsIT #14585

Open

Contributor

github-actions bot commented Jul 2, 2025

❌ Gradle check result for 9c9a819: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 3, 2025

❌ Gradle check result for 582046a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

opensearch-ci-bot mentioned this pull request

[AUTOCUT] Gradle Check Flaky Test Report for IndexServiceTests #14407

Open

Contributor

github-actions bot commented Jul 7, 2025

❌ Gradle check result for 9304305: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 8, 2025

❌ Gradle check result for bad98f0: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

This was referenced Jul 8, 2025

[AUTOCUT] Gradle Check Flaky Test Report for RecoveryWhileUnderLoadIT #14509

Open

[AUTOCUT] Gradle Check Flaky Test Report for RestoreShallowSnapshotV2IT #16658

Open

Contributor

github-actions bot commented Jul 8, 2025

❌ Gradle check result for bf5672e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 8, 2025

❌ Gradle check result for 3ee163b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 8, 2025

❌ Gradle check result for 615fe3d:

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 8, 2025

❌ Gradle check result for 09b4bf0: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for fde8d04: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

opensearch-ci-bot mentioned this pull request

[AUTOCUT] Gradle Check Flaky Test Report for SearchRestCancellationIT #14311

Open

Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for efbfcfc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

andrevandeven force-pushed the time-fetch branch from efbfcfc to b1a271e Compare

July 9, 2025 05:45

Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for b1a271e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for 980f63d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Contributor

github-actions bot commented Jul 9, 2025

❌ Gradle check result for 13a1a98: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Andre van de Ven and others added 4 commits

July 9, 2025 09:39


          added timing for entire fetch phase

2302f3c

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>


          added more granular fetch profiling

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>


          added tests

629ba22

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>


          ran spotless

e3f8f44

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>


          spotless

bdc5fc9

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>

Contributor

github-actions bot commented Jul 30, 2025

❌ Gradle check result for bdc5fc9: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?


          fixed script fields test yml test after change in timing type name fo…

d03e2e7

…r next_reader

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>

Contributor

github-actions bot commented Jul 30, 2025

❌ Gradle check result for d03e2e7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

andrevandeven added 3 commits

July 30, 2025 20:40


          fixed issue where profile node time wasn't inclusive of child timings

418e7f8

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>


          removed profiling of inner hits fetch and aggregation fetch

da82a68

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>


          removed profiling of inner hits subphase

6683ccc

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>

Contributor

github-actions bot commented Jul 31, 2025

❌ Gradle check result for 6683ccc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

This was referenced Jul 30, 2025

[AUTOCUT] Gradle Check Flaky Test Report for AzureBlobStoreRepositoryTests #14291

Open

[AUTOCUT] Gradle Check Flaky Test Report for AwarenessAllocationIT #17930

Open


          empty commit

cf75be8

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>

andrevandeven requested a review from msfroh

July 31, 2025 17:38

Contributor Author

andrevandeven commented Jul 31, 2025

@msfroh I've decided to create a META issue to break this into smaller changes. I've disabled profiling of InnerHits and TopHitsAggregation for now, but have documented them in an issue within the Meta. Would appreciate if you could review again!

andrevandeven requested a review from finnegancarroll

July 31, 2025 17:39

Contributor

github-actions bot commented Jul 31, 2025

✅ Gradle check result for cf75be8: SUCCESS

bowenlan-amzn approved these changes

View reviewed changes

bowenlan-amzn reviewed

View reviewed changes

server/src/main/java/org/opensearch/search/profile/fetch/FlatFetchProfileTree.java Show resolved Hide resolved


          throw error if no parent specified for start subphase

f6d05d4

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>

Contributor

msfroh commented Jul 31, 2025

@msfroh I've decided to create a META issue to break this into smaller changes. I've disabled profiling of InnerHits and TopHitsAggregation for now, but have documented them in an issue within the Meta. Would appreciate if you could review again!

Yeah, that makes sense! They're complicated special cases, so punting them to their own task sounds right. 👍

msfroh approved these changes

View reviewed changes

Contributor

github-actions bot commented Jul 31, 2025

✅ Gradle check result for f6d05d4: SUCCESS

Contributor Author

andrevandeven commented Jul 31, 2025

@msfroh Thanks for the review! I've resolved remaining comments, so should be good to go.

jainankitk approved these changes

View reviewed changes

Contributor

jainankitk left a comment

Thanks @andrevandeven for this useful contribution!

jainankitk merged commit e082c30 into opensearch-project:main

31 checks passed

opensearch-ci-bot mentioned this pull request

[AUTOCUT] Gradle Check Flaky Test Report for TransferManagerBlobContainerReaderTests #18872

Open

sunqijun1 pushed a commit to sunqijun1/OpenSearch that referenced this pull request


          Add Fetch Phase to Search Profile (opensearch-project#18664)

a84fb44

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>
Signed-off-by: Andre van de Ven <113951599+andrevandeven@users.noreply.github.com>
Co-authored-by: Andre van de Ven <andrevdv@amazon.com>
Signed-off-by: sunqijun.jun <sunqijun.jun@bytedance.com>

tandonks pushed a commit to tandonks/OpenSearch that referenced this pull request


          Add Fetch Phase to Search Profile (opensearch-project#18664)

8709a7c

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>
Signed-off-by: Andre van de Ven <113951599+andrevandeven@users.noreply.github.com>
Co-authored-by: Andre van de Ven <andrevdv@amazon.com>

BrewTestBot mentioned this pull request

opensearch 3.2.0 Homebrew/homebrew-core#234146

Merged

vinaykpud pushed a commit to vinaykpud/OpenSearch that referenced this pull request


          Add Fetch Phase to Search Profile (opensearch-project#18664)

0d4cd5f

Signed-off-by: Andre van de Ven <andrebvandeven@gmail.com>
Signed-off-by: Andre van de Ven <113951599+andrevandeven@users.noreply.github.com>
Co-authored-by: Andre van de Ven <andrevdv@amazon.com>

opensearch-ci-bot mentioned this pull request

[AUTOCUT] Gradle Check Flaky Test Report for FileBasedIngestionSingleNodeTests #19723

Closed

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

Reviewers

msfroh msfroh approved these changes

jainankitk jainankitk approved these changes

anasalkouz Awaiting requested review from anasalkouz anasalkouz is a code owner

andrross Awaiting requested review from andrross andrross is a code owner

ashking94 Awaiting requested review from ashking94 ashking94 is a code owner

Bukhtawar Awaiting requested review from Bukhtawar Bukhtawar is a code owner

CEHENKLE Awaiting requested review from CEHENKLE CEHENKLE is a code owner

cwperks Awaiting requested review from cwperks cwperks is a code owner

dbwiddis Awaiting requested review from dbwiddis dbwiddis is a code owner

gbbafna Awaiting requested review from gbbafna gbbafna is a code owner

jed326 Awaiting requested review from jed326 jed326 is a code owner

kotwanikunal Awaiting requested review from kotwanikunal kotwanikunal is a code owner

mch2 Awaiting requested review from mch2 mch2 is a code owner

owaiskazi19 Awaiting requested review from owaiskazi19 owaiskazi19 is a code owner

reta Awaiting requested review from reta reta is a code owner

Rishikesh1159 Awaiting requested review from Rishikesh1159 Rishikesh1159 is a code owner

sachinpkale Awaiting requested review from sachinpkale sachinpkale is a code owner

saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner

shwetathareja Awaiting requested review from shwetathareja shwetathareja is a code owner

sohami Awaiting requested review from sohami sohami is a code owner

VachaShah Awaiting requested review from VachaShah

finnegancarroll Awaiting requested review from finnegancarroll

+1 more reviewer

bowenlan-amzn bowenlan-amzn approved these changes

Labels

distributed framework documentation pending enhancement lucene