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

Fix formatting cumulative memory in Web UI #12596

Merged
merged 2 commits into from
May 31, 2022

Conversation

losipiuk
Copy link
Member

Value for cumulative memory of a query was previously reported using
following format:

"4.33T seconds". It is not obviouls that it represents integer of memory
coinsumption over time.
This commit changes format to:
"4.33TB*seconds"

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Web UI
* Improve readability of Cumulative User Memory on Query Details page. ({issue}`issuenumber`)

@losipiuk
Copy link
Member Author

image

@losipiuk
Copy link
Member Author

Previously:

image

@findepi
Copy link
Member

findepi commented May 30, 2022

Can you please make labeller apply the ui label?

</td>
{taskRetriesEnabled &&
<td className="info-failed">
{formatDataSizeBytes(query.queryStats.failedCumulativeUserMemory / 1000.0) + " seconds"}
{formatDataSize(query.queryStats.failedCumulativeUserMemory / 1000.0) + "*seconds"}
Copy link
Member

Choose a reason for hiding this comment

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

MB*s ? (MBs would be correct, but not clear)

Copy link
Member Author

Choose a reason for hiding this comment

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

I started with MBs but yeah, I think it is not clear.
Elsewhere we use full seconds (not an abbreviation), so I guess this is fine.

Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

optional

@losipiuk losipiuk added needs-docs This pull request requires changes to the documentation ui Web UI and removed needs-docs This pull request requires changes to the documentation labels May 31, 2022
losipiuk added 2 commits May 31, 2022 09:05
Value for cumulative memory of a query was previously reported using
following format:

"4.33T seconds". It is not obviouls that it represents integer of memory
coinsumption over time.
This commit changes format to:
"4.33TB*seconds"
@losipiuk losipiuk force-pushed the lo/fix-formating-cm branch from 5ccddb0 to 5fd2387 Compare May 31, 2022 07:05
@losipiuk losipiuk removed the ui Web UI label May 31, 2022
@losipiuk
Copy link
Member Author

Can you please make labeller apply the ui label?

👍

@losipiuk losipiuk merged commit 954125e into trinodb:master May 31, 2022
@github-actions github-actions bot added this to the 383 milestone May 31, 2022
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.

3 participants