-
Notifications
You must be signed in to change notification settings - Fork 25.4k
ESQL: Add times to topn status #131555
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
ESQL: Add times to topn status #131555
Conversation
Adds times to the TopNOperator status, specifically the nanoseconds spent receiving the values and the nanoseconds spent emitting the values: ``` { "operator" : "TopNOperator[count=0/1000, elementTypes=[BYTES_REF, DOUBLE], encoders=[UTF8TopNEncoder, DefaultSortable], sortOrders=[SortOrder[channel=1, asc=false, nullsFirst=true]]]", "status" : { "receive_nanos" : 193415, <--- this row "emit_nanos" : 61, <--- and this row "occupied_rows" : 0, "ram_bytes_used" : 4296, "ram_used" : "4.1kb", "pages_received" : 1, "pages_emitted" : 1, "rows_received" : 1000, "rows_emitted" : 1000 } } ```
Pinging @elastic/es-analytical-engine (Team:Analytics) |
@@ -548,9 +553,11 @@ public Page getOutput() { | |||
if (output == null || output.hasNext() == false) { | |||
return null; | |||
} | |||
long start = System.nanoTime(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should capture the emit time in finish()
(toPages
) method instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking more closely - of course. Wow. I wasn't paying attention. Thanks!
builder.field("receive_nanos", receiveNanos); | ||
builder.field("emit_nanos", emitNanos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other statuses also add the "X_time" with the readable time. For example:
Lines 436 to 439 in 59df1bf
builder.field("hash_nanos", hashNanos); | |
if (builder.humanReadable()) { | |
builder.field("hash_time", TimeValue.timeValueNanos(hashNanos)); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Makes sense. Something I'd have asked for I think.
@@ -423,6 +427,7 @@ public void addInput(Page page) { | |||
pagesReceived++; | |||
rowsReceived += page.getPositionCount(); | |||
} | |||
receiveNanos += System.nanoTime() - start; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in the finally, in case of exception (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May as well, yeah. I think the exception here is generally terminal to the process and we don't return the status, but may as well put it there so it's possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks Nik!
…king * upstream/main: (100 commits) Term vector API on stateless search nodes (elastic#129902) TEST Fix ThreadPoolMergeSchedulerStressTestIT testMergingFallsBehindAndThenCatchesUp (elastic#131636) Add inference.put_custom rest-api-spec (elastic#131660) ESQL: Fewer serverless docs in tests (elastic#131651) Skip search on indices with INDEX_REFRESH_BLOCK (elastic#129132) Mute org.elasticsearch.indices.cluster.RemoteSearchForceConnectTimeoutIT testTimeoutSetting elastic#131656 [jdk] Resolve EA OpenJDK builds to our JDK archive (elastic#131237) Add optimized path for intermediate values aggregator (elastic#131390) Correctly handling download_database_on_pipeline_creation within a pipeline processor within a default or final pipeline (elastic#131236) Refresh potential lost connections at query start for `_search` (elastic#130463) Add template_id to patterned-text type (elastic#131401) Integrate LIKE/RLIKE LIST with ReplaceStringCasingWithInsensitiveRegexMatch rule (elastic#131531) [ES|QL] Add doc for the COMPLETION command (elastic#131010) ESQL: Add times to topn status (elastic#131555) ESQL: Add asynchronous pre-optimization step for logical plan (elastic#131440) ES|QL: Improve generative tests for FORK [130015] (elastic#131206) Update index mapping update privileges (elastic#130894) ESQL: Added Sample operator NamedWritable to plugin (elastic#131541) update `kibana_system` to grant it access to `.chat-*` system index (elastic#131419) Clarify heap size configuration (elastic#131607) ...
…-tracking * upstream/main: (44 commits) Term vector API on stateless search nodes (elastic#129902) TEST Fix ThreadPoolMergeSchedulerStressTestIT testMergingFallsBehindAndThenCatchesUp (elastic#131636) Add inference.put_custom rest-api-spec (elastic#131660) ESQL: Fewer serverless docs in tests (elastic#131651) Skip search on indices with INDEX_REFRESH_BLOCK (elastic#129132) Mute org.elasticsearch.indices.cluster.RemoteSearchForceConnectTimeoutIT testTimeoutSetting elastic#131656 [jdk] Resolve EA OpenJDK builds to our JDK archive (elastic#131237) Add optimized path for intermediate values aggregator (elastic#131390) Correctly handling download_database_on_pipeline_creation within a pipeline processor within a default or final pipeline (elastic#131236) Refresh potential lost connections at query start for `_search` (elastic#130463) Add template_id to patterned-text type (elastic#131401) Integrate LIKE/RLIKE LIST with ReplaceStringCasingWithInsensitiveRegexMatch rule (elastic#131531) [ES|QL] Add doc for the COMPLETION command (elastic#131010) ESQL: Add times to topn status (elastic#131555) ESQL: Add asynchronous pre-optimization step for logical plan (elastic#131440) ES|QL: Improve generative tests for FORK [130015] (elastic#131206) Update index mapping update privileges (elastic#130894) ESQL: Added Sample operator NamedWritable to plugin (elastic#131541) update `kibana_system` to grant it access to `.chat-*` system index (elastic#131419) Clarify heap size configuration (elastic#131607) ...
Adds times to the TopNOperator status, specifically the nanoseconds spent receiving the values and the nanoseconds spent emitting the values: