Skip to content

fix: set nginx worker_processes to fixed value to prevent OOMKill - #1241

Open
pranithraoibm wants to merge 1 commit into
IBM:mainfrom
pranithraoibm:nginx-fix
Open

fix: set nginx worker_processes to fixed value to prevent OOMKill#1241
pranithraoibm wants to merge 1 commit into
IBM:mainfrom
pranithraoibm:nginx-fix

Conversation

@pranithraoibm

Copy link
Copy Markdown
Contributor

nginx worker_processes auto reads /proc/cpuinfo (node CPU count) instead of the pod's cpu limit, causing pods to spawn 48 worker processes on a 48-vCPU node and get OOMKilled.

Set worker_processes to a fixed value aligned with each pod's cpu limit:

  • catalog (500m limit): 2
  • chatbot (50m limit): 1
  • digitize (50m limit): 1

Also reduced worker_connections from 1024 to 256 to match the reduced worker footprint.

Signed-off-by: Pranith Rao <pranith.rao@ibm.com>

@Shubhamag12 Shubhamag12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm
one question: are these configurations tested?

Comment on lines +1 to 8
worker_processes 1;

error_log /var/log/nginx/error.log warn;
pid /tmp/nginx.pid;

events {
worker_connections 1024;
worker_connections 256;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How did we come up with this number?
How exactly will this be impacted?. Did we do thorough validations and ensure no issues?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pranithraoibm can you please explain what is this for?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pranithraoibm please put a comment to change the numbers when someone changes the limits. Better add comment at both the places.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So this change was something that Bob had suggested

worker_connections defines the maximum number of simultaneous connections per worker process.
Total max connections = worker_processes × worker_connections
In our case, it is 512 for catalog and 256 for chatbot and digitize

256 was chosen because it is proportionate to what the pod can actually handle given its CPU limit (500m and 50m).
So this right-sizes the connection pool to match the worker count and CPU budget, reducing unnecessary memory reservation.

Just wanted to highlight this, we can revert back if this seems as a unnecessary improvement.

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.

5 participants