proposal: cmd/go: build option to set env variable defaults

[networkimprov]

Users are asking for a way to hard-code runtime environment variables, because their end-users cannot set them on app invocation.

I suggest a go build/test/run command-line option. Its use needn't be restricted to Go-specific variables. Existence of a variable in the environment at runtime would override any build-time default.

go build -env 'GODEBUG=asyncpreemptoff=1,madvdontneed=1;MYVAR=x' // semicolon separated list

#37569 (comment) @elgatito
#40722 (comment) @gwillem

I considered a new stdlib API, but that opens the possibility of conflict between packages. A way to prevent that is to restrict the API to package main, but that seems unprecedented.

EDIT: fix example to use semicolon delimiter

[elgatito]

I would like to add that priorities for options setters should be:
Environment -> Build -> Defaults.

[rsc]

If users are asking for this, it means we have failed at the way we configure programs.
Let's correct our mistakes instead of embracing and building atop our failures.
What specific values do users want to hard-code into the binaries and why?
What should we do to make it so those don't need to be set?

On top of that, program configuration belongs in programs not on build command lines.
Instead of go build -env 'GOGC=50' a program can call runtime.SetGCPercent(50).

If the problem is bugs that can be disabled with GODEBUG settings,
perhaps we should have debug.Set("asyncpreempt", "1") or something like that
and let programs call it during init.

/cc @aclements

[elgatito]

@rsc

What specific values do users want to hard-code into the binaries and why?

In my case, I need to set values for "asyncpreemptoff" and "madvdontneed" for specific builds, no difference, is it "set on build" or "set by code".

As a develop, I want to be sure program runs the way I plan it, giving users ability to override that with environment settings.
Not always we have the control how the program is ran.

[aclements]

These are called GODEBUG because they're meant for debugging. If you have to set them programmatically, it suggests we've done something wrong (in the case of madvdontneed, maybe Linux did something wrong, but that's another story). It's possible we could put some of these in the runtime/debug package as @rsc suggested, but I'd like to understand better why you need to change them first.

[elgatito]

@aclements In my case I have build targets for linux hardware, running on kernel 3.14, and without madvdontneed it acts differently.

I would accept any way, build options or runtime/debug calls.

[aclements]

Sorry, but that doesn't really answer my question. Why do you need these options? How does it behave differently and why is it important that you change that?

[networkimprov]

Some end-users cannot set env vars (stated at top). GODEBUG can't investigate or workaround problems in those cases.

Besides the issues linked at top, 1.14 & 1.15 break CIFS & FUSE. Fixes are in for 1.16 but won't be backported.
#38836 #39237 #40846

[rsc]

@networkimprov, lucky for us this build option wouldn't be backported either, so it isn't any better than a general fix to the EINTR problem. Let's solve that instead.

[rsc]

It sounds like the real problem is that there are two runtime bugs that have not been adequately addressed, and too many people are working around them by setting GODEBUG settings. The two workarounds that are being overused are madvdontneed=1, for various perceived or actual out-of-memory conditions on Linux; and asyncpreemptoff=1, for various problems with spurious EINTR returns from Linux.

For madvdontneed=1, I believe the state of the world is as @aclements summarized in #33376 (comment). Go is doing the same thing as Java and Node here. It should not be necessary to set that. It is unfortunate that Linux tools like top/htop do not subtract out the lazyfree total from RSS in the display, but the OS knows what it can take away and will do that rather than OOM. That comment also says that container memory limits understand what Go is doing here too, so this should not be causing container OOMs either. If someone is seeing problems here, we need to get more information about their exact setup to help understand it better. Hence the (unanswered) questions to @elgatito above and more importantly on #37569. If there are other cases where using MADV_FREE is causing OOMs, please file details in a different issue and we will try to get to the bottom of that.

For asyncpreemptoff=1, the problem is that preemption has caused some (arguably buggy) kernel behavior returning EINTR where we weren't checking for it. And now I believe we are checking. If there are more places to check, let's fix those.

In both cases, fixing the specific bugs is more effective and will be easier than adding a new build option we will have to support for all time.

Let's fix the real bugs.

[networkimprov]

Certainly fix the bugs, but why discount the value of letting end-users run apps with a preset GODEBUG (or other env) value? Why not a runtime/debug API for it?

That is the only way you can possibly get debug info from certain sites.

[rsc]

It's just more API surface that we are stuck with forever, and it's API surface that shouldn't even exist.

[rsc]

Based on the discussion above, this seems like a likely decline.

[networkimprov]

You just got another inquiry about this: #41818 (comment)

[elgatito]

@rsc I understand, you want to stay clear and avoid having environment variables hell, but, again, with the change of MADV_FREE behavior in Go, when running on hosts with old kernels (like 3.17) we get OOM from time to time.
I'm not sure it's related to Go really, but I'd like to control such things in my binary, without shell scripts.

[blinkinglight]

I think, each env variable should have method with its own documentation, instead of comment block in env variables. Because how madvdontneed=1 and production related to word goDEBUG? Also, I think, that defaults should be reviewed and who really needs performance ought to have ability to tune it.

[rsc]

@elgatito, the answer for MADV_FREE is being discussed in #37569. It may be that we should just stop trying to use it, at least on Android, but possibly everywhere. That's separate from this discussion.

@blinkinglight, these are meant to be temporary knobs, not permanent API. If lots of people need one, we're doing something wrong and we should fix it, as I commented above.

[rsc]

No change in consensus, so declined.

[networkimprov]

A workaround on unix might be to check os.GetEnv() for the necessary variable, set it with os.SetEnv(), then restart the app:

unix.Exec(os.Args[0], os.Args[1:], os.Environ())

https://godoc.org/golang.org/x/sys/unix#Exec

[neilalexander]

For what it's worth, MADV_FREE/MADV_DONTNEED does create a rather significant perceptual problem with resource usage. When users look at top/htop and see the resident set size inflated and seemingly never decreasing, the first thing they usually do is point the finger towards the developers because "your program is using all of my memory"!

It would be nice to have a way to choose between perception vs optimisation in cases like this that don't require setting environment variables.

Not to mention that, for things built using gobind/gomobile, environment variables aren't exactly straight-forward.

[gopherbot]

Change https://golang.org/cl/267100 mentions this issue: runtime: default to MADV_DONTNEED on Linux

[networkimprov]

@aclements @rsc @ianlancetaylor

Here is another case #41099 (comment) describing the need to disable async preemption for all instances of an application (it trips up a required third-party library).

In that light it may be worth reconsidering this proposal...