Correctly check for vm.overcommit_memory == 0 #10841

jtru · 2022-06-09T15:22:49Z

A regression caused by #10636 (released in 7.0.1) causes Redis startup warning about overcommit to be missing when needed and printed when not.

Also, using atoi() to convert the string's value to an integer cannot discern
between an actual 0 (zero) having been read, or a conversion error.

Behavior after fix (warns correctly):

$ sudo sysctl -w vm.overcommit_memory=0
vm.overcommit_memory = 0
$ ./src/redis-server 2>&1 | grep -F vm.overcommit
69889:M 09 Jun 2022 17:19:22.922 # WARNING WARNING overcommit_memory is set to 0! Background save may fail under low memory condition. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
^C
$ sudo sysctl -w vm.overcommit_memory=1
vm.overcommit_memory = 1
$ ./src/redis-server 2>&1 | grep -F vm.overcommit
^C

Behavior as of 7.0.1 (bogus warning):

$ sudo sysctl -w vm.overcommit_memory=0
vm.overcommit_memory = 0
$ ./src/redis-server 2>&1 | grep -F vm.overcommit
^C
$ sudo sysctl -w vm.overcommit_memory=1
vm.overcommit_memory = 1
$ ./src/redis-server 2>&1 | grep -F vm.overcommit
70145:M 09 Jun 2022 17:20:51.930 # WARNING WARNING overcommit_memory is set to 0! Background save may fail under low memory condition. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
^C

redis-server should warn upon startup if the vm.overcommit_memory sysctl is set to 0, as this could have adverse effects under memory pressure. The value is parsed from an ASCII string retrieved from a file in /proc. Using atoi() to convert the string's value to an integer cannot discern between an actual 0 (zero) having been read, or a conversion error having occurred. Also, the check as implemented had its intended logic reversed, failing to warn for an actual value of 0, but emitting the warning in all other potential cases (vm.overcommit_memory is not a boolean value). Using strtol() instead of atoi() and comparing its return value to 0 should work.

oranagra · 2022-06-09T17:28:31Z

looks like a regression caused by #10636 (released in 7.0.1).
used to do ==0 which was dropped.

jtru · 2022-06-12T16:08:49Z

7.0.2 exhibits the same problem as 7.0.1:

$ sysctl vm.overcommit_memory
vm.overcommit_memory = 0
$ ./src/redis-server 
7220:C 12 Jun 2022 18:07:21.784 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
7220:C 12 Jun 2022 18:07:21.784 # Redis version=7.0.2, bits=64, commit=05833959, modified=0, pid=7220, just started
7220:C 12 Jun 2022 18:07:21.784 # Warning: no config file specified, using the default config. In order to specify a config file use ./src/redis-server /path/to/redis.conf
7220:M 12 Jun 2022 18:07:21.785 * Increased maximum number of open files to 10032 (it was originally set to 1024).
7220:M 12 Jun 2022 18:07:21.785 * monotonic clock: POSIX clock_gettime
                _._                                                  
           _.-``__ ''-._                                             
      _.-``    `.  `_.  ''-._           Redis 7.0.2 (05833959/0) 64 bit
  .-`` .-```.  ```\/    _.,_ ''-._                                  
 (    '      ,       .-`  | `,    )     Running in standalone mode
 |`-._`-...-` __...-.``-._|'` _.-'|     Port: 6379
 |    `-._   `._    /     _.-'    |     PID: 7220
  `-._    `-._  `-./  _.-'    _.-'                                   
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |           https://redis.io       
  `-._    `-._`-.__.-'_.-'    _.-'                                   
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |                                  
  `-._    `-._`-.__.-'_.-'    _.-'                                   
      `-._    `-.__.-'    _.-'                                       
          `-._        _.-'                                           
              `-.__.-'                                               

7220:M 12 Jun 2022 18:07:21.785 # Server initialized
7220:M 12 Jun 2022 18:07:21.785 * Loading RDB produced by version 7.0.2
7220:M 12 Jun 2022 18:07:21.785 * RDB age 58 seconds
7220:M 12 Jun 2022 18:07:21.785 * RDB memory usage when created 0.82 Mb
7220:M 12 Jun 2022 18:07:21.785 * Done loading RDB, keys loaded: 0, keys expired: 0.
7220:M 12 Jun 2022 18:07:21.785 * DB loaded from disk: 0.000 seconds
7220:M 12 Jun 2022 18:07:21.785 * Ready to accept connections
^C7220:signal-handler (1655050043) Received SIGINT scheduling shutdown...
7220:M 12 Jun 2022 18:07:23.699 # User requested shutdown...
7220:M 12 Jun 2022 18:07:23.699 * Saving the final RDB snapshot before exiting.
7220:M 12 Jun 2022 18:07:23.727 * DB saved on disk
7220:M 12 Jun 2022 18:07:23.727 # Redis is now ready to exit, bye bye...

$ sudo sysctl -w vm.overcommit_memory=1
vm.overcommit_memory = 1
$ ./src/redis-server 
7235:C 12 Jun 2022 18:07:28.782 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
7235:C 12 Jun 2022 18:07:28.782 # Redis version=7.0.2, bits=64, commit=05833959, modified=0, pid=7235, just started
7235:C 12 Jun 2022 18:07:28.782 # Warning: no config file specified, using the default config. In order to specify a config file use ./src/redis-server /path/to/redis.conf
7235:M 12 Jun 2022 18:07:28.783 * Increased maximum number of open files to 10032 (it was originally set to 1024).
7235:M 12 Jun 2022 18:07:28.783 * monotonic clock: POSIX clock_gettime
                _._                                                  
           _.-``__ ''-._                                             
      _.-``    `.  `_.  ''-._           Redis 7.0.2 (05833959/0) 64 bit
  .-`` .-```.  ```\/    _.,_ ''-._                                  
 (    '      ,       .-`  | `,    )     Running in standalone mode
 |`-._`-...-` __...-.``-._|'` _.-'|     Port: 6379
 |    `-._   `._    /     _.-'    |     PID: 7235
  `-._    `-._  `-./  _.-'    _.-'                                   
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |           https://redis.io       
  `-._    `-._`-.__.-'_.-'    _.-'                                   
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |                                  
  `-._    `-._`-.__.-'_.-'    _.-'                                   
      `-._    `-.__.-'    _.-'                                       
          `-._        _.-'                                           
              `-.__.-'                                               

7235:M 12 Jun 2022 18:07:28.783 # Server initialized
7235:M 12 Jun 2022 18:07:28.783 # WARNING overcommit_memory is set to 0! Background save may fail under low memory condition. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
7235:M 12 Jun 2022 18:07:28.783 * Loading RDB produced by version 7.0.2
7235:M 12 Jun 2022 18:07:28.783 * RDB age 5 seconds
7235:M 12 Jun 2022 18:07:28.783 * RDB memory usage when created 0.82 Mb
7235:M 12 Jun 2022 18:07:28.783 * Done loading RDB, keys loaded: 0, keys expired: 0.
7235:M 12 Jun 2022 18:07:28.783 * DB loaded from disk: 0.000 seconds
7235:M 12 Jun 2022 18:07:28.783 * Ready to accept connections
^C7235:signal-handler (1655050050) Received SIGINT scheduling shutdown...
7235:M 12 Jun 2022 18:07:30.193 # User requested shutdown...
7235:M 12 Jun 2022 18:07:30.193 * Saving the final RDB snapshot before exiting.
7235:M 12 Jun 2022 18:07:30.218 * DB saved on disk
7235:M 12 Jun 2022 18:07:30.218 # Redis is now ready to exit, bye bye...

Please merge this trivial fix - iI've kept its diff minimal on purpose.

oranagra · 2022-06-12T16:18:25Z

@jtru I'm sorry. I messed up and cleared the label confusing it with the other issue.

oranagra · 2022-06-12T16:49:52Z

@jtru i edited the top comment for the purpose of release notes (and commit comment), please confirm it's OK.

jtru · 2022-06-12T17:48:04Z

Sure thing, 100% fine with me :) Thanks for merging!

Yossifsolman · 2022-06-14T20:03:38Z

Yeah @oranagra i got here now, this exactly what i have too.

@jtru couple of questions maybe can help ( i am experience the same issue as you ):

Each repository, you download your Redis 7.0.1?
You use cPanel? (or other server management program for Linux?)

jtru · 2022-06-15T07:20:55Z

I downloaded the official release tarball.
No. Also, this is irrelevant to the issue at hand.

To benefit from the fix (it is actually just cosmetic; the warning is spurious - redis will not change behavior in any other way than not displaying the warning when it is not warranted to do so) wait for the next release including it, and install/run that. Until then, you don't need to do anything other than ignore it, once you checked that your vm.overcommit setting is on whatever you need and expect it to.

Yossifsolman · 2022-06-20T06:29:35Z

OK, I found something, i suspect Redis actually not getting wrong and this not a bug, this is a cloud level issue (Google Cloud and AWS):

from what i see, it can be overcommit because it's turn off on a Cloud level ( i use Google cloud )

i will check on it and get back to you guys.
If you have useful information how to activate this on the Google Cloud Platform, its be much easier.

oranagra · 2022-06-20T07:05:25Z

CPU overcommit is not the same as virtual memory overcommit.
The memory overcommit is a feature of the OS, has nothing to do with the cloud provider.

Yossifsolman · 2022-06-20T12:31:43Z

You right its a node, i see it now in the docs:
https://cloud.google.com/compute/docs/nodes/overcommitting-cpus-sole-tenant-vms#console

A regression caused by redis#10636 (released in 7.0.1) causes Redis startup warning about overcommit to be missing when needed and printed when not. Also, using atoi() to convert the string's value to an integer cannot discern between an actual 0 (zero) having been read, or a conversion error.

oranagra added the 7.0-must-have label Jun 9, 2022

This was referenced Jun 10, 2022

[CRASH] redis 7.0.1 crashes on start after upgrading from 7.0.0 #10846

Closed

Fix crash when overcommit_memory is inaccessible #10848

Merged

oranagra removed the 7.0-must-have label Jun 11, 2022

oranagra approved these changes Jun 12, 2022

View reviewed changes

oranagra added the 7.0-must-have label Jun 12, 2022

oranagra merged commit 9f3b410 into redis:unstable Jun 12, 2022

oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Jun 12, 2022

oranagra mentioned this pull request Jun 13, 2022

redis.x86_64 0:7.0.1-1.el7.remi upgrade (i believe new Redis version) #10857

Closed

jtru deleted the fix-overcommit-sysctl-detection branch June 15, 2022 07:17

sundb mentioned this pull request Jun 30, 2022

Redis and vm.overcommit_memory #10906

Closed

This was referenced Jul 2, 2022

[BUG] Redis 7.0.2 vm.overcommit log warning is incorrect #10916

Closed

[BUG] 7.0.1, 7.0.2 syscheck.c checkOvercommit() if (atoi(buf)) { "msg" } -> Not working #10926

Closed

oranagra mentioned this pull request Jul 11, 2022

Release 7.0.3 #10962

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Correctly check for vm.overcommit_memory == 0 #10841

Correctly check for vm.overcommit_memory == 0 #10841

Uh oh!

jtru commented Jun 9, 2022 •

edited by oranagra

Loading

Uh oh!

oranagra commented Jun 9, 2022

Uh oh!

jtru commented Jun 12, 2022 •

edited

Loading

Uh oh!

oranagra commented Jun 12, 2022

Uh oh!

oranagra commented Jun 12, 2022

Uh oh!

jtru commented Jun 12, 2022

Uh oh!

Yossifsolman commented Jun 14, 2022

Uh oh!

jtru commented Jun 15, 2022

Uh oh!

Yossifsolman commented Jun 20, 2022 •

edited

Loading

Uh oh!

oranagra commented Jun 20, 2022

Uh oh!

Yossifsolman commented Jun 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Correctly check for vm.overcommit_memory == 0 #10841

Correctly check for vm.overcommit_memory == 0 #10841

Uh oh!

Conversation

jtru commented Jun 9, 2022 • edited by oranagra Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Behavior after fix (warns correctly):

Behavior as of 7.0.1 (bogus warning):

Uh oh!

oranagra commented Jun 9, 2022

Uh oh!

jtru commented Jun 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oranagra commented Jun 12, 2022

Uh oh!

oranagra commented Jun 12, 2022

Uh oh!

jtru commented Jun 12, 2022

Uh oh!

Yossifsolman commented Jun 14, 2022

Uh oh!

jtru commented Jun 15, 2022

Uh oh!

Yossifsolman commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oranagra commented Jun 20, 2022

Uh oh!

Yossifsolman commented Jun 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jtru commented Jun 9, 2022 •

edited by oranagra

Loading

jtru commented Jun 12, 2022 •

edited

Loading

Yossifsolman commented Jun 20, 2022 •

edited

Loading