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

mem input: Treat buffer/cache memory as available, rather than used #1617

Closed
wants to merge 2 commits into from

Conversation

nigels-com
Copy link
Contributor

Signed-off-by: Nigel Stewart nigels@nigels.com

I noticed this in the course of testing with a Raspberry Pi 1 with only half a GiB of RAM.

While free is reporting a smallish about of used and free memory, a good portion is used for buffering and caching. fluent-bit is reporting this "bufferram" as used, rather than available.

$ free
              total        used        free      shared  buff/cache   available
Mem:         443844       92616       71456        5260      279772      281844
Swap:        102396       14592       87804
[0] memory: [1570365689.889683667, {"Mem.total"=>443844, "Mem.used"=>365440, "Mem.free"=>78404, "Swap.total"=>102396, "Swap.used"=>14592, "Swap.free"=>87804}]

Signed-off-by: Nigel Stewart <nigels@nigels.com>
@nigels-com
Copy link
Contributor Author

Originally this plugin was using MemAvailable from /proc/meminfo

95cf9bf44 (Eduardo Silva 2016-01-19 15:20:31 -0800 125)     /* Available Memory */
95cf9bf44 (Eduardo Silva 2016-01-19 15:20:31 -0800 126)     tmp = field(buf, "MemAvailable");
95cf9bf44 (Eduardo Silva 2016-01-19 15:20:31 -0800 127)     if (!tmp) {
95cf9bf44 (Eduardo Silva 2016-01-19 15:20:31 -0800 128)         return -1;
95cf9bf44 (Eduardo Silva 2016-01-19 15:20:31 -0800 129)     }
95cf9bf44 (Eduardo Silva 2016-01-19 15:20:31 -0800 130)     *available = atoll(tmp);
2f882b808 (Eduardo Silva 2016-10-19 09:57:39 -0600 131)     flb_free(tmp);
MemTotal:         443844 kB
MemFree:           55892 kB
MemAvailable:     292072 kB
Buffers:           51516 kB
Cached:           215504 kB

@nigels-com
Copy link
Contributor Author

nigels-com commented Oct 6, 2019

Ah, the problem seems to run deeper. To get the cached memory, even free appears to be parsing /proc/meminfo

https://gitlab.com/procps-ng/procps/blob/master/proc/sysinfo.c

Leave this with me.

@nigels-com
Copy link
Contributor Author

See also: #1194

@nigels-com
Copy link
Contributor Author

nigels-com commented Oct 6, 2019

htop is also looking at /proc/meminfo
https://github.com/hishamhm/htop/blob/master/linux/LinuxProcessList.c#L918

Signed-off-by: Nigel Stewart <nigels@nigels.com>
@nigels-com
Copy link
Contributor Author

nigels-com commented Oct 7, 2019

Yeah, that's more like it, I think.

[0] memory: [1570412546.957227628, {"Mem.total"=>443844, "Mem.used"=>160312, "Mem.free"=>283532, "Swap.total"=>102396, "Swap.used"=>16128, "Swap.free"=>86268}]
$ free --kilo
              total        used        free      shared  buff/cache   available
Mem:         454496       89944       46714        8228      317837      290287
Swap:        104853       16515       88338
MemTotal:         443844 kB
MemFree:           47660 kB
MemAvailable:     285528 kB
Buffers:           52572 kB
Cached:           218900 kB
SwapCached:         1380 kB
Active:           175144 kB
Inactive:         164412 kB
Active(anon):      11496 kB
Inactive(anon):    64624 kB
Active(file):     163648 kB
Inactive(file):    99788 kB

@nokute78
Copy link
Collaborator

nokute78 commented Oct 7, 2019

How about adding new option ?
It will append a new field “Mem.available” and its default is false.

I think it is better for compatibility.

@nigels-com
Copy link
Contributor Author

I understand the concern about compatibility. But I don't think "getting it right" should be opt-in. It's a bug that Mem.free isn't what most people would expect it mean. (It's misleading because Linux tends to run with little "free" memory by design, it's simply not a useful runtime metric, ignoring buffers and caching)

@nigels-com
Copy link
Contributor Author

I think the compatible thing to do for this is continue reporting free as before, but add available in addition to the the current fields. Sound good?

@edsiper
Copy link
Member

edsiper commented Oct 8, 2019

sgtm

@nokute78
Copy link
Collaborator

nokute78 commented Oct 8, 2019

Append field

Some output plugins may be affected .
(e.g. DB schema needs to be changed )

By the way ,MemAvailable is supported from Linux Kernel 3.14.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34e431b0ae398fc54ea69ff85ec700722c9da773

I believe we should take care legacy kernel (e.g. CentOS 6.X which is supported until 2020.)

@nokute78
Copy link
Collaborator

nokute78 commented Oct 8, 2019

legacy kernel support

Just FYI
Beats tries to calculate.
https://github.com/elastic/beats/blob/master/vendor/github.com/elastic/go-sysinfo/providers/linux/memory_linux.go#L64

@edsiper
Copy link
Member

edsiper commented May 5, 2020

what's the status of this PR ?

@edsiper edsiper added the waiting-for-user Waiting for more information, tests or requested changes label May 5, 2020
@edsiper edsiper self-assigned this May 5, 2020
@nigels-com
Copy link
Contributor Author

It seems like the next step for this is to do what memory_linux.go does, as mentioned above.
Is there consensus that this would likely to be accepted?

@edsiper
Copy link
Member

edsiper commented May 18, 2020

sounds good to me to use those approaches:

  1. Use MemAvailable or
  2. do the simple calculation if we are in an older Kernel

@edsiper
Copy link
Member

edsiper commented Jun 30, 2020

ping

@edsiper
Copy link
Member

edsiper commented Jul 16, 2020

@nigels-com pls rebase on top of git master for review.

@edsiper
Copy link
Member

edsiper commented Nov 24, 2020

@nigels-com ping

@edsiper
Copy link
Member

edsiper commented Dec 13, 2020

ping

@nigels-com
Copy link
Contributor Author

Will revisit, fell off my radar.

@cveilleux
Copy link
Contributor

I have cleaned-up/rebased on top of master here: #5237

@cveilleux
Copy link
Contributor

This PR should be closed. There are now 2 competing PR's that are more up-to-date and complete:

#3092 parses /proc/meminfo and collects more metrics, with some calculations as a fallback for older kernels. The new metrics collected could be concidered a breaking change though.
#5237 is a simpler approach that also parses /proc/meminfo but does not change the field names/metrics reported. It just sends a more accurate metric if it is available, with a fallback to the old, broken behavior on old kernels.

@nigels-com nigels-com closed this Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-user Waiting for more information, tests or requested changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants