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

v1.5.0 I am still getting that the CPU Quota is not defined. #49

Closed
dprotaso opened this issue Apr 6, 2022 · 9 comments · Fixed by #50
Closed

v1.5.0 I am still getting that the CPU Quota is not defined. #49

dprotaso opened this issue Apr 6, 2022 · 9 comments · Fixed by #50

Comments

@dprotaso
Copy link
Contributor

dprotaso commented Apr 6, 2022

Originally posted by @ardan-bkennedy in #44 (comment)

This change has not fixed the problem. I switched my code from "github.com/emadolsky/automaxprocs/maxprocs" to v1.5.0 and I am still getting that the CPU Quota is not defined.

I noticed that in v1.5.0

https://github.com/uber-go/automaxprocs/blob/v1.5.0/internal/runtime/cpu_quota_linux.go#L35

func CPUQuotaToGOMAXPROCS(minValue int) (int, CPUQuotaStatus, error) {[Jason Lai, 5 years ago: • Import `automaxprocs/x/runtime`](https://sourcegraph.com/github.com/uber-go/automaxprocs/-/commit/572239b375b1ca6b76babeb8b0ee139dc35fd4d8)
	cgroups, err := newQueryer()
	if err != nil {
		return -1, CPUQuotaUndefined, err
	}

	quota, defined, err := cgroups.CPUQuota()
	if !defined || err != nil {
		return -1, CPUQuotaUndefined, err
	}

	maxProcs := int(math.Floor(quota))
	if minValue > 0 && maxProcs < minValue {
		return minValue, CPUQuotaMinUsed, nil
	}
	return maxProcs, CPUQuotaUsed, nil
}

var (
	_newCgroups2 = cg.NewCGroups2ForCurrentProcess
	_newCgroups  = cg.NewCGroupsForCurrentProcess
)

func newQueryer() (queryer, error) {
	cgroups, err := _newCgroups2()
	if err == nil {
		return cgroups, nil
	}
	if errors.Is(err, cg.ErrNotV2) {
		return _newCgroups()
	}
	return nil, err
}

In the emadolsky repo, it's working. I don't know enough to tell you why.

func CPUQuotaToGOMAXPROCS(minValue int) (int, CPUQuotaStatus, error) {
	var quota float64
	var defined bool
	var err error

	isV2, err := cg.IsCGroupV2()
	if err != nil {
		return -1, CPUQuotaUndefined, err
	}

	if isV2 {
		quota, defined, err = cg.CPUQuotaV2()
		if !defined || err != nil {
			return -1, CPUQuotaUndefined, err
		}
	} else {
		cgroups, err := cg.NewCGroupsForCurrentProcess()
		if err != nil {
			return -1, CPUQuotaUndefined, err
		}

		quota, defined, err = cgroups.CPUQuota()
		if !defined || err != nil {
			return -1, CPUQuotaUndefined, err
		}
	}

	maxProcs := int(math.Floor(quota))
	if minValue > 0 && maxProcs < minValue {
		return minValue, CPUQuotaMinUsed, nil
	}
	return maxProcs, CPUQuotaUsed, nil
}
@dprotaso dprotaso changed the title This change has not fixed the problem. I switched my code from "github.com/emadolsky/automaxprocs/maxprocs" to v1.5.0 and I am still getting that the CPU Quota is not defined. v1.5.0 I am still getting that the CPU Quota is not defined. Apr 6, 2022
@dprotaso
Copy link
Contributor Author

dprotaso commented Apr 6, 2022

cc @abhinav @mway

@ardan-bkennedy
Copy link

ardan-bkennedy commented Apr 6, 2022

The code is public domain if someone wants to look at what I'm running. I am using the latest version v0.12 of KIND and the recommended container.

https://github.com/ardanlabs/service

https://github.com/ardanlabs/service/blob/master/app/services/sales-api/main.go

@emadolsky
Copy link
Contributor

@ardan-bkennedy - Yes the merged version seems to have issues. Probably, I will be able to take a look at it 'til the end of the day.

@ardan-bkennedy
Copy link

Brilliant and thanks for all the attention on this. Very much appreciated.

@abhinav
Copy link
Contributor

abhinav commented Apr 6, 2022

Thanks for the report @emadolsky @ardan-bkennedy!
Our apologies. Looks like we broke it somewhere in the refactoring/testing.

The point where it decides between cgroups2 and 1 is here:

cgroups, err := _newCgroups2()
if err == nil {
return cgroups, nil
}
if errors.Is(err, cg.ErrNotV2) {
return _newCgroups()
}
return nil, err

But I think the issue is here:

newMountPoint = func(mp *MountPoint) error {
isV2 = mp.FSType == _cgroupv2FSType && mp.MountPoint == _cgroupv2MountPoint
return nil
}

We made a small mistake there with the refactoring: we turned "if any of the mount points meet this condition" to "if the last mount point meets this condition". The tests didn't catch it because the test mountinfo file has a single entry.

We'll get this fixed and push a patch release ASAP.

abhinav pushed a commit that referenced this issue Apr 6, 2022
In #44, we accidentally turned v2 mountpoint detection
from "are any of the mountpoints v2 to "is the last mountpoint v2".
This fixes that condition and updates the test case to catch that in
the future.

Resolves #49
@abhinav
Copy link
Contributor

abhinav commented Apr 6, 2022

Fix released. Thanks for flagging this @emadolsky @ardan-bkennedy!

@ardan-bkennedy
Copy link

Brilliant. Can't wait to test it later today!!

@ardan-bkennedy
Copy link

YES!!! This is working!!! THANK YOU!

wenjianhn added a commit to wenjianhn/website that referenced this issue Feb 16, 2023
Add an example of uber-go/automaxprocs which is used by lots of applications to avoid CPU throttling.

Related to uber-go/automaxprocs#49
DonatoHorn pushed a commit to DonatoHorn/website that referenced this issue Jun 25, 2023
Add an example of uber-go/automaxprocs which is used by lots of applications to avoid CPU throttling.

Related to uber-go/automaxprocs#49
@shcw
Copy link

shcw commented Sep 13, 2024

I'm still having this problem with 1.5.3 😭😭😭😭😭😭

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 a pull request may close this issue.

5 participants