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

server: handle new behavior of setrlimit on macOS #37705

Merged
merged 1 commit into from
May 21, 2019

Conversation

benesch
Copy link
Contributor

@benesch benesch commented May 21, 2019

macOS's getrlimit can return a garbage hard limit for the number of open
files--that is, a hard limit which is higher than what setrlimit(2) will
accept. Since Go 1.12's syscall.Setrlimit actually delegates to the C
library's setrlimit, rather than making a system call directly,
Cockroach can fail to start because its call to syscall.Setrlimit now
fails.

This commit fixes the problem in two ways:

  1. It prevents a failed setrlimit call from stopping the entire
    server startup process, unless the original rlimit is really
    beneath the minimum allowable rlimit.

  2. It provides a macOS wrapper for syscall.Getrlimit that returns the
    actual hard limit, thus preventing the setrlimit call from failing
    in the first place.

Fix #37685.

Release note: None

macOS's getrlimit can return a garbage hard limit for the number of open
files--that is, a hard limit which is higher than what setrlimit(2) will
accept. Since Go 1.12's syscall.Setrlimit actually delegates to the C
library's setrlimit, rather than making a system call directly,
Cockroach can fail to start because its call to syscall.Setrlimit now
fails.

This commit fixes the problem in two ways:

  1. It prevents a failed setrlimit call from stopping the entire
     server startup process, unless the original rlimit is really
     beneath the minimum allowable rlimit.

  2. It provides a macOS wrapper for syscall.Getrlimit that returns the
     actual hard limit, thus preventing the setrlimit call from failing
     in the first place.

Fix cockroachdb#37685.

Release note: None
@benesch benesch requested review from bdarnell and a team May 21, 2019 19:02
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@bdarnell bdarnell left a comment

Choose a reason for hiding this comment

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

Thanks!

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@benesch
Copy link
Contributor Author

benesch commented May 21, 2019

bors r=bdarnell

craig bot pushed a commit that referenced this pull request May 21, 2019
37582: sql: fix multiplication incorrect of float and interval r=mjibson a=hueypark

Fixes #37540

Release note (bug fix): fix multiplication incorrect of float and interval

37705: server: handle new behavior of setrlimit on macOS r=bdarnell a=benesch

macOS's getrlimit can return a garbage hard limit for the number of open
files--that is, a hard limit which is higher than what setrlimit(2) will
accept. Since Go 1.12's syscall.Setrlimit actually delegates to the C
library's setrlimit, rather than making a system call directly,
Cockroach can fail to start because its call to syscall.Setrlimit now
fails.

This commit fixes the problem in two ways:

  1. It prevents a failed setrlimit call from stopping the entire
     server startup process, unless the original rlimit is really
     beneath the minimum allowable rlimit.

  2. It provides a macOS wrapper for syscall.Getrlimit that returns the
     actual hard limit, thus preventing the setrlimit call from failing
     in the first place.

Fix #37685.

Release note: None

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
@craig
Copy link
Contributor

craig bot commented May 21, 2019

Build succeeded

@craig craig bot merged commit e777ba0 into cockroachdb:master May 21, 2019
@benesch benesch deleted the ulimit branch May 21, 2019 20:27
zbeekman added a commit to Homebrew/formula-patches that referenced this pull request May 22, 2019
 - Issue with v19 & go 1.12
 - `setrlimit` changed causing crash if `kern.maxfiles` is too low.
 - See cockroachdb/cockroach#37705
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.

server: Improve Setrlimit error handling
3 participants