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

doc: syscall: document Setrlimit behavior change on Go 1.12 on macOS #30401

Closed
karalabe opened this issue Feb 26, 2019 · 13 comments
Closed

doc: syscall: document Setrlimit behavior change on Go 1.12 on macOS #30401

karalabe opened this issue Feb 26, 2019 · 13 comments
Labels
CherryPickApproved Used during the release process for point releases Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Milestone

Comments

@karalabe
Copy link
Contributor

We've noticed a while back that when setting the file descriptor allowance on macos, Go restricts it to a lower value than requested:

package main

import (
	"fmt"
	"syscall"
)

func main() {
	// Get the current file descriptor limit
	var limit syscall.Rlimit
	if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &limit); err != nil {
		panic(err)
	}
	// Try to update the limit to the max allowance
	limit.Cur = limit.Max
	if err := syscall.Setrlimit(syscall.RLIMIT_NOFILE, &limit); err != nil {
		panic(err)
	}
	// Try to get the limit again and see where it's at
	if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &limit); err != nil {
		panic(err)
	}
	fmt.Println(limit.Cur)
}

The above code on Linux works as expected, setting the limit to the max returned by the syscall package.

On macos with Go 1.11.5 (and prior) it silently set it to the minimum of limit.Max and 10240. We had to patch our code to explicitly check the actual value set instead of relying on no-error-returned since we falsely alloted more fds to databases than available.

On macos with Go 1.12, our code again started to blow up, now returning invalid argument errors instead of the silent capping previously.

I'd probably say both behaviors are bad, but the new one is a lot worse than the previous. Now it only tells us it's invalid, but unless you know the magic number defined in the your own kernel version, there's no way for user code to figure out how many fds are actually available.

@ianlancetaylor
Copy link
Member

CC @randall77

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Feb 26, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.12.1 milestone Feb 26, 2019
@randall77
Copy link
Contributor

Yes, this is unfortunate. We switched to using the implementation of getrlimit in libSystem.dylib instead of raw system calls, and that's the behavior we get. In particular, the new implementation lies to us about the maximum value available.
I had to modify the TestRlimit with code like this:

	if runtime.GOOS == "darwin" && set.Cur > 10240 {
		// The max file limit is 10240, even though
		// the max returned by Getrlimit is 1<<63-1.
		// This is OPEN_MAX in sys/syslimits.h.
		set.Cur = 10240
	}

I suppose we could make our implementation of Getrlimit incorporate this fix internally. But my view of the syscall package is that you get what the OS provides, warts and all. And one day Apple may fix this issue (or change OPEN_MAX) and then any patch we have is obsolete.

You can always binary search for the largest allowable value. Yuck.

@randall77
Copy link
Contributor

Note that this is not just a Go problem - C people run into similar issues:

https://postfix-devel.postfix.narkive.com/kVL334x7/mac-os-x-and-setrlimit-2

@karalabe
Copy link
Contributor Author

Ah yes, I wasn't particularly complaining that Go is doing something wrong (given the restrictions), just trying to figure out what the best solution is. I guess my main complaint wasn't that Go works one way or the other, rather that it changed it behavior, breaking existing code.

We can fix it to handle both cases (i.e. hard code that magic number), just would like to know which solution will Go stay with in the end so we don't have to do a 3rd fixup round for Go 1.12.1 :)

@ianlancetaylor
Copy link
Member

Going forward on Darwin Go is always going to call the C function.

@jeffallen
Copy link
Contributor

jeffallen commented Mar 7, 2019

I found that on Go 1.11.5, this code:

func init() {
	raiseFdLimit = func() {
		var rLimit syscall.Rlimit
		err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit)
		if err != nil {
			log.Fatal("Error Getting Rlimit ", err)
		}

		if rLimit.Cur < rLimit.Max {
			rLimit.Cur = rLimit.Max
			err = syscall.Setrlimit(syscall.RLIMIT_NOFILE, &rLimit)
			if err != nil {
				log.Warn("Error Setting Rlimit:", err)
			}
		}

		err = syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit)
		log.Info("File descriptor limit is:", rLimit.Cur)
	}
}

reports File descriptor limit is: 24576. On Go 1.12 it reports invalid argument. So I've added the workaround from above (#30401 (comment)) but instead of 10240, I've used 24576 and that returns the behavior of my server compiled with Go 1.12 to the same as it was on Go 1.11.5. (@randall77: MacOS version 10.14.3 (18D109))

The manpage says: setrlimit() now returns with errno set to EINVAL in places that historically succeeded. It no longer accepts "rlim_cur = RLIM_INFINITY" for RLIM_NOFILE. Use "rlim_cur = min(OPEN_MAX, rlim_max)".

I searched for OPEN_MAX in /Application/Xcode.app and found that it is 10240, even though the observed behavior is 24576 works. 🤷‍♂️

@randall77
Copy link
Contributor

@jeffallen What Mac OS are you running?
I can't get ask for more than 10240 on Sierra (10.12.6) without getting an invalid argument error.

@jeffallen
Copy link
Contributor

jeffallen commented Mar 7, 2019

Here is an excellent explanation: https://unix.stackexchange.com/questions/350776/setting-os-x-macos-bsd-maxfile

It says this could be a problem on FreeBSD and OpenBSD as well.

Another useful reference, showing that reading the setrlimit manpage and paying any attention whatsoever to OPEN_MAX would be not a good idea: https://julio.meroh.net/2019/01/open-files-limit-macos-and-the-jvm.html

@bep
Copy link
Contributor

bep commented Apr 6, 2019

For what it's worth;

On my Mojave 10.14.4 on Go 1.12.2, my tests tell me that I can set values in Setrlimit <= the value of limit as defined in /Library/LaunchDaemons/limit.maxfiles.plist (I assume that 10240 is the default?).

This can be set for the current session with something ala:

sudo launchctl limit maxfiles 1234 200000

In the above example, I get invalid argument for any argument to Setrlimit> 1234.

If I run Setrlimit as sudo, I can use any value.

@andybons andybons modified the milestones: Go1.12.3, Go1.12.4, Go1.12.5 Apr 8, 2019
@andybons andybons modified the milestones: Go1.12.5, Go1.12.6 May 6, 2019
grailbot pushed a commit to grailbio/reflow that referenced this issue May 13, 2019
Summary:
This commit implements a workaround to an issue [[ golang/go#30401 |  introduced in Go 1.12 ]].

The behavior of `Setrlimit` changed to no longer succeed when the requested limit exceeded the maximum.  (It used to succeed, clamping the value to the allowed maximum).

Test Plan: Manual testing

Reviewers: smahadevan, pgopal, marius

Reviewed By: marius

Maniphest Tasks: T17776

Differential Revision: https://phabricator.grailbio.com/D27211

fbshipit-source-id: 8be13f0
@andybons andybons changed the title syscall: Setrlimit behavior changed on Go 1.12 on macos doc: syscall: document Setrlimit behavior chang on Go 1.12 on macos Jul 16, 2019
@andybons andybons changed the title doc: syscall: document Setrlimit behavior chang on Go 1.12 on macos doc: syscall: document Setrlimit behavior change on Go 1.13 on macOS Jul 16, 2019
reltuk pushed a commit to dolthub/dolt that referenced this issue Jul 24, 2019
… ...) on Darwin.

It turns out rlim_max from getrlimit(RLIMIT_NOFILE, ...) is not a valid value
for rlim_cur in setrlimit(RLIMIT_NOFILE, ...) on Darwin. Golang used to silently
fix this for us, but it stopped doing it. See
golang/go#30401.

Take reasonable steps to work around it for now.
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/188237 mentions this issue: doc/go1.13: document change in syscall.Setrlimit behavior

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/188357 mentions this issue: [release-branch.go1.12] doc/go1.12: document change in syscall.Setrlimit behavior

gopherbot pushed a commit that referenced this issue Jul 31, 2019
…mit behavior

Fixes #30401

Change-Id: I7b5035ffc7333c746d4e31563df26ff4f934dfc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/188237
Reviewed-by: Emmanuel Odeke <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
(cherry picked from commit fe8a866)
Reviewed-on: https://go-review.googlesource.com/c/go/+/188357
@dmitshur dmitshur modified the milestones: Go1.13, Go1.12.9 Aug 15, 2019
@dmitshur dmitshur changed the title doc: syscall: document Setrlimit behavior change on Go 1.13 on macOS doc: syscall: document Setrlimit behavior change on Go 1.12 on macOS Aug 15, 2019
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. CherryPickApproved Used during the release process for point releases and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Aug 15, 2019
@frank-segui
Copy link

I found that on Go 1.11.5, this code:

func init() {
	raiseFdLimit = func() {
		var rLimit syscall.Rlimit
		err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit)
		if err != nil {
			log.Fatal("Error Getting Rlimit ", err)
		}

		if rLimit.Cur < rLimit.Max {
			rLimit.Cur = rLimit.Max
			err = syscall.Setrlimit(syscall.RLIMIT_NOFILE, &rLimit)
			if err != nil {
				log.Warn("Error Setting Rlimit:", err)
			}
		}

		err = syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit)
		log.Info("File descriptor limit is:", rLimit.Cur)
	}
}

reports File descriptor limit is: 24576. On Go 1.12 it reports invalid argument. So I've added the workaround from above (#30401 (comment)) but instead of 10240, I've used 24576 and that returns the behavior of my server compiled with Go 1.12 to the same as it was on Go 1.11.5. (@randall77: MacOS version 10.14.3 (18D109))

The manpage says: setrlimit() now returns with errno set to EINVAL in places that historically succeeded. It no longer accepts "rlim_cur = RLIM_INFINITY" for RLIM_NOFILE. Use "rlim_cur = min(OPEN_MAX, rlim_max)".

I searched for OPEN_MAX in /Application/Xcode.app and found that it is 10240, even though the observed behavior is 24576 works. 🤷‍♂️

I experienced the same. Found that sysctl kern.maxfilesperproc was set to 24576. I increased kern.maxfilesperproc and was able to increase rLimit.Cur to the new value.

This was in Mojave 10.14.6

madjar added a commit to madjar/nix-build-uncached that referenced this issue Aug 24, 2020
Running on osx, the attempt to change the rlimit fails with "invalid argument".

It appears that the value of rlimit.Max is `9223372036854775807`, and that
trying to set to that value fails. More on this at
golang/go#30401.

We fix this by setting the value to the somewhat magic 24576 when on osx (darwin).
Mic92 pushed a commit to Mic92/nix-build-uncached that referenced this issue Aug 25, 2020
Running on osx, the attempt to change the rlimit fails with "invalid argument".

It appears that the value of rlimit.Max is `9223372036854775807`, and that
trying to set to that value fails. More on this at
golang/go#30401.

We fix this by setting the value to the somewhat magic 24576 when on osx (darwin).
Mic92 pushed a commit to Mic92/nix-build-uncached that referenced this issue Aug 25, 2020
Running on osx, the attempt to change the rlimit fails with "invalid argument".

It appears that the value of rlimit.Max is `9223372036854775807`, and that
trying to set to that value fails. More on this at
golang/go#30401.

We fix this by setting the value to the somewhat magic 24576 when on osx (darwin).
@golang golang locked and limited conversation to collaborators Jan 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Projects
None yet
Development

No branches or pull requests

10 participants