Skip to content

Improve the documentation for two memory counters#23126

Merged
yingsu00 merged 1 commit intoprestodb:masterfrom
yingsu00:memoryDoc
Jul 5, 2024
Merged

Improve the documentation for two memory counters#23126
yingsu00 merged 1 commit intoprestodb:masterfrom
yingsu00:memoryDoc

Conversation

@yingsu00
Copy link
Contributor

@yingsu00 yingsu00 commented Jul 2, 2024

Description

Following the discussion of 0.288 release notes, I'm making these changes to clarify the two memory counters meanings. They are query-memory-gb and query-reserved-memory-gb.

Impact

Help the user to understand these counters better

Test Plan

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.
== NO RELEASE NOTE ==

@yingsu00 yingsu00 requested review from a team and steveburnett as code owners July 2, 2024 20:57
@yingsu00 yingsu00 mentioned this pull request Jul 2, 2024
36 tasks
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Nits of formatting, nothing else. Let me know if my suggestions are not correct, please.

xiaoxmeng
xiaoxmeng previously approved these changes Jul 2, 2024
Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@yingsu00 LGTM. Thanks for the improvement!

/// The minimal amount of memory capacity in bytes reserved for each query
/// memory pool.
/// The amount of memory in bytes reserved for each query memory pool. When
/// a query is trying to allocate memory from the reserved space whose size is
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/is trying/tries/

/// Specifies the total amount of memory in GB reserved for the queries on
/// a single worker node. A query can only allocate from this reserved space if
/// 1) the non-reserved space in "query-memory-gb" is used up; and 2) the amount
/// it is trying to get is less than 'memory-pool-reserved-capacity'.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/is trying/tries/

@yingsu00
Copy link
Contributor Author

yingsu00 commented Jul 3, 2024

@steveburnett Thanks for reviewing! I have updated the PR with your comment addressed. Can you please review again?

steveburnett
steveburnett previously approved these changes Jul 3, 2024
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)
Pull updated branch, new local docs build, looks good. Thanks!

@yingsu00
Copy link
Contributor Author

yingsu00 commented Jul 3, 2024

Thanks @xiaoxmeng and @steveburnett for reviewing!
@steveburnett if this looks ok to you, will you be able to approve this PR? THanks!

Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local docs build, looks good. Thanks!

@yingsu00 yingsu00 merged commit 72f4af6 into prestodb:master Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants