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

Lazy computation of some fields in QueryMetadata and QueryStatistics #21445

Conversation

radek-kondziolka
Copy link
Contributor

@radek-kondziolka radek-kondziolka commented Apr 8, 2024

Description

  1. Let's compute QueryCompletedEvent only once, even if there is multiple event listeners.
  2. Let's compute QueryMetadata#paylaod and QueryStatistics#operatorSummaries lazily because of heaviness (CPU and memory) of these computation.

Motivation

A lot of CPU cycles released

Fixes a regression introduced in 7f30476 (#12968) -- QueryCompletedEvent was being created multiple times, and it's sometimes quite expensive.

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

Computation of QueryMetadata#payload and QueryStatistics#operatorSummaries
is a heavy operation. This change causes that they will be computated
at most once for multiple event listeners and computation will be lazy -
most event listeners do not need this kind of information.
@cla-bot cla-bot bot added the cla-signed label Apr 8, 2024
@radek-kondziolka radek-kondziolka requested a review from sopel39 April 8, 2024 09:27
@sopel39
Copy link
Member

sopel39 commented Apr 9, 2024

do you want to take a look @electrum @martint ?

cc @findepi

@findepi
Copy link
Member

findepi commented Apr 9, 2024

Added this to the PR description:

Fixes a regression introduced in 7f30476 (#12968) -- QueryCompletedEvent was being created multiple times, and it's sometimes quite expensive.

@findepi
Copy link
Member

findepi commented Apr 9, 2024

pt (default, suite-cassandra, ) cannot pass, since the PT server artifact already expired
we would need to run all tests again to make this one work, probably not worth it

@findepi findepi merged commit 25f86e3 into trinodb:master Apr 9, 2024
93 of 95 checks passed
@github-actions github-actions bot added this to the 445 milestone Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants