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

Use ulimit to check the open files soft limit #66

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Use ulimit to check the open files soft limit #66

merged 2 commits into from
Sep 12, 2024

Conversation

tagatac
Copy link
Owner

@tagatac tagatac commented Sep 12, 2024

What is changing: Use the value returned from ulimit -n as the open files soft limit instead of the value returned from the syscall getrlimit.

Why this change is being made: Since Go 1.19, during initialization, Go programs unconditionally set a high open files soft limit for themselves without modifying the system-wide defaults. This soft limit also does not apply to subprocesses run via the exec package. Because wkhtmltopdf is run as a subprocess, the soft limit set by the Go program during initialization does not give us any information about the limits that will be applied to wkhtmltopdf. Two approaches would be to

  1. always call setrlimit on every run so that the soft limit will apply to subprocesses, or
  2. use ulimit to check the soft limit that will be applied to wkhtmltopdf.

I chose to implement approach 2. to reduce the number of syscalls.

More details:

Related issue(s): Fixes #63

Follow-up changes needed: None

Is the change completely covered by unit tests? If not, why not?: Yes

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.98%. Comparing base (8d1118a) to head (7d8af1b).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/bagoup/main.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
+ Coverage   93.76%   93.98%   +0.21%     
==========================================
  Files          14       14              
  Lines         786      798      +12     
==========================================
+ Hits          737      750      +13     
+ Misses         45       44       -1     
  Partials        4        4              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tagatac tagatac merged commit b3b93a0 into main Sep 12, 2024
4 checks passed
@tagatac tagatac deleted the ulimit branch September 12, 2024 17:12
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.

pdfFile: Too many open files
1 participant