hostagent: increase nofile limit

[vasileknik76]

Default nofile limit in the standard terminal in macOS is 256. You can see it with ulimit -n.

So when I tried to open over 240 connections to 127.0.0.1, the ssh process crashed.

Default limit is too low for a vm that can run many applications.

I suggest to use the same approach as in golang - setting an increased limit for the lima needs.

PR relates on all types of networks, but only for the macOS.

I understand that users can increase the limit themselves using ulimit -n <N>. But the regular user expects to get a working environment without exploring such low-level features.

[jandubois]

I don't understand why this change is necessary. Go 1.19+ should already increase the limit by itself. Is this not the case?

[vasileknik76]

Go 1.19+ increase the limit only for itself. But in exec limit returns to the original value. It means that any (*exec.Cmd).Run will be runned with the old limit.

An explicit call syscall.Setrlimit modifies atomic origRlimitNofile and the above condition does not affect the new exec's

[jandubois]

Go 1.19+ increase the limit only for itself.

It does? That seems to contradict the 1.19 release notes, which state:

One impact of this change is that Go programs that in turn execute very old C programs in child processes may run those programs with too high a limit.

So if this change is indeed still necessary, then we obviously should raise the limit ourselves too. I'm generally prejudiced against using init functions for this because it makes error reporting/logging harder, but maybe we can assume that this call will never fail... 😄

[vasileknik76]

This is actual for both Go 1.19 and Go 1.21.

A simple program for demonstration of limits:

package main

import (
	"fmt"
	"log"
	"os/exec"
	"syscall"
)

func printLimit() {
	cmd := exec.Command("ulimit", "-n")
	if out, err := cmd.CombinedOutput(); err != nil {
		log.Fatal(err)
	} else {
		fmt.Println(string(out))
	}
}

func main() {
	var limit syscall.Rlimit
	if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &limit); err == nil && limit.Cur != limit.Max {
		limit.Cur = limit.Max

		fmt.Println("ulimit before setrlimit")
		printLimit()
		_ = syscall.Setrlimit(syscall.RLIMIT_NOFILE, &limit)
		fmt.Println("ulimit after setrlimit")
		printLimit()

	}
}

On my M1 with go 1.19 it prints:

ulimit before setrlimit
256

ulimit after setrlimit
unlimited

In the worst case, during initialization there will be no increase in limits and everything will remain as before.

I have already applied the same patch to our Lima-based devenv and it showed good results. It seems that Lima will also benefit from this.

[jandubois]

Thanks, LGTM

[jandubois]

Should this also be applied on Linux then? I realize the default will be higher, but is there any reason not to raise to the max limit there as well?

[vasileknik76]

Should this also be applied on Linux then? I realize the default will be higher, but is there any reason not to raise to the max limit there as well?

I think this can be applied to Linux as well. But different Linux distros may have different defaults. And since I'm not sure about Linux, I decided to make a patch for darwin first.

BTW: thanks for the quick review.

[jandubois]

I think this can be applied to Linux as well. But different Linux distros may have different defaults. And since I'm not sure about Linux, I decided to make a patch for darwin first.

I think the same reasoning should apply to Linux, regardless of what the default soft limit is. I'll leave it to @AkihiroSuda so determine what we should do (and if it should be part of this PR if we want to apply the same change on both).

I've restarted failed tests because I don't see how the failures can be caused by this PR.

[AkihiroSuda]

Maybe the error should be logged via logrus.Debug?

[AkihiroSuda]

I also wonder this should be moved to https://github.com/lima-vm/lima/blob/master/cmd/limactl/hostagent.go , as it affects reverse-sshfs too IIUC.

[AkihiroSuda]

(It covers non-Darwin too)

[vasileknik76]

Yes, it affects reverse-sshfs too.

I moved the code to hostagent.go and make the fuction comment more wide.
Also I added debug logging if an error occurs

[AkihiroSuda]

Thanks

[AkihiroSuda]

This line is not needed

[vasileknik76]

Without the constraint i got an error:

$ GOOS=windows go build ./cmd/limactl
# github.com/lima-vm/lima/pkg/hostagent
pkg/hostagent/hostagent_windows.go:3:6: adjustNofileRlimit redeclared in this block
        pkg/hostagent/hostagent_unix.go:15:6: other declaration of adjustNofileRlimit
pkg/hostagent/hostagent_unix.go:16:20: undefined: syscall.Rlimit
pkg/hostagent/hostagent_unix.go:17:20: undefined: syscall.Getrlimit
pkg/hostagent/hostagent_unix.go:17:38: undefined: syscall.RLIMIT_NOFILE
pkg/hostagent/hostagent_unix.go:21:17: undefined: syscall.Setrlimit
pkg/hostagent/hostagent_unix.go:21:35: undefined: syscall.RLIMIT_NOFILE