Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

count buffer/cache into MemInfo #550

Merged
merged 3 commits into from
Jun 24, 2021
Merged

count buffer/cache into MemInfo #550

merged 3 commits into from
Jun 24, 2021

Conversation

critical27
Copy link
Contributor

@critical27 critical27 commented May 31, 2021

Total = Used + Free + Buffer/Cache

Maybe need to take buffer/cache into consideration, here a screenshot when I do some test.

image

Graph will keep logging something like Used memory(211303932KB) hits the high watermark(0.800000) of total system memory(263856036KB)..

PR is still in verification.

@critical27 critical27 added the ready-for-testing PR: ready for the CI test label May 31, 2021
@critical27 critical27 requested review from yixinglu and a team May 31, 2021 04:23
Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@panda-sheep panda-sheep left a comment

Choose a reason for hiding this comment

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

Good!

@yixinglu
Copy link
Contributor

We should count the physical memory usage, not include buffer. Here freeInKB() returns the same result as the free column of free -h.

More information refer to https://stackoverflow.com/questions/63166/how-to-determine-cpu-and-memory-consumption-from-inside-a-process

@yixinglu yixinglu added the need to discuss PR: need to discuss the current PR label May 31, 2021
@critical27
Copy link
Contributor Author

We should count the physical memory usage, not include buffer. Here freeInKB() returns the same result as the free column of free -h.

More information refer to https://stackoverflow.com/questions/63166/how-to-determine-cpu-and-memory-consumption-from-inside-a-process

Could have discuss later. Some known issues when I write data by importer:

  1. Failed to write data once pressure if enough
  2. Some interface could bypass the check later, show hosts will fail as well if high watermark is reached.

If take buffer into consideration, it will solve these problems.

@critical27 critical27 removed the ready-for-testing PR: ready for the CI test label Jun 2, 2021
@yixinglu
Copy link
Contributor

yixinglu commented Jun 7, 2021

We should count the physical memory usage, not include buffer. Here freeInKB() returns the same result as the free column of free -h.
More information refer to https://stackoverflow.com/questions/63166/how-to-determine-cpu-and-memory-consumption-from-inside-a-process

Could have discuss later. Some known issues when I write data by importer:

  1. Failed to write data once pressure if enough
  2. Some interface could bypass the check later, show hosts will fail as well if high watermark is reached.

If take buffer into consideration, it will solve these problems.

Good point for question 1. Now you can update this config to 1.0 before importing and restore the config when finishing data loading.

In query engine, we have ignored the checks for most executors except query related executors and if show hosts still report this error, that's a bug.

@CLAassistant
Copy link

CLAassistant commented Jun 17, 2021

CLA assistant check
All committers have signed the CLA.

@yixinglu yixinglu added ready-for-testing PR: ready for the CI test and removed need to discuss PR: need to discuss the current PR labels Jun 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants