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

Message change on upgrade to 1.2.0 #20

Open
eccles opened this issue Nov 22, 2019 · 1 comment
Open

Message change on upgrade to 1.2.0 #20

eccles opened this issue Nov 22, 2019 · 1 comment

Comments

@eccles
Copy link

eccles commented Nov 22, 2019

Go 1.13 and go modules are being used.

I have used v1.1.0 using Set with a logger. v1.1.0 tells me that GOMAXPROCS has been reduced from 4 to 1 because of Cgroups CPUQuta restrictions which is what I expect.

 {"level":"info","ts":1574415010.177917,"caller":"logger/logger.go:77","msg":"Cores allocation","GOMAXPROCS":4}                                                                                                      
 {"level":"info","ts":1574415010.1784537,"caller":"maxprocs/maxprocs.go:47","msg":"maxprocs: Updating GOMAXPROCS=1: determined from CPU quota"}  

I tried v1.2.0 and now the log message tells me GOMAXPROCS is set to one because this is the minimum. This seems incorrect. Note also that the log message has changed format.

 maxprocs/maxprocs.go:47 maxprocs: Updating GOMAXPROCS=1: using minimum allowed GOMAXPROCS.

I have reverted to v1.1.0.

The code fragment is:

    logger.Sugar.Infow(
            "Cores allocation",
            "GOMAXPROCS", runtime.GOMAXPROCS(-1),
        )
        undoMaxProcs, err = maxprocs.Set(maxprocs.Logger(Sugar.Infof))
        if err != nil {
            logger.Sugar.Errorw(
                "Error for automaxprocs",
                "err", err,
            )
        } 

Another improvement would be to emit in the log message the old value of GOMAXPROCS as well as what it is changed to.

@prashantv
Copy link
Contributor

There was a change in 1.2.0 which is likely related:

#13

Fixed quota clamping to always round down rather than up; Rather than guaranteeing constant throttling at saturation, instead assume that the fractional CPU was added as a hedge for factors outside of Go's scheduler.

In the previous version, a CPU quota of less than a core (e.g., 0.5) would use the ceiling and round up to 1. However, the above change uses the floor, so the calculated GOMAXPROCS would be 0, which we then are clamping up to the minimum value of 1.

There have been no logging changes between 1.1.0 and 1.2.0 so they might be related to the logger you pass in.

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

No branches or pull requests

2 participants