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

cgroup: retry file writes on EINTR errors #3102

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

adunham-stripe
Copy link
Contributor

@adunham-stripe adunham-stripe commented Jun 29, 2020

After deploying a version of gvisor built with Go 1.14, we're seeing errors setting up cgroups (we manually run runsc via runsc run, which creates the cgroup). This turns out to be a known issue with Go: golang/go#38033. Given that the fix won't be backported, we should retry writes that may fail with EINTR.

This is also what runc does: opencontainers/runc#2258

@googlebot googlebot added the cla: yes CLA has been signed label Jun 29, 2020
Copy link
Member

@prattmic prattmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

copybara-service bot pushed a commit that referenced this pull request Jul 8, 2020
After deploying a version of gvisor built with Go 1.14, we're seeing errors setting up cgroups (we manually run `runsc` via `runsc run`, which creates the cgroup). This turns out to be a known issue with Go: golang/go#38033. Given that the [fix won't be backported](golang/go#39026 (comment)), we should retry writes that may fail with EINTR.

This is also what runc does: opencontainers/runc#2258

FUTURE_COPYBARA_INTEGRATE_REVIEW=#3102 from stripe:andrew/cgroup-eintr 079123b
PiperOrigin-RevId: 320183771
@prattmic
Copy link
Member

Ping @nlacasse this never got merged.

// Retry writes on EINTR; see:
// https://github.com/golang/go/issues/38033
for {
err := ioutil.WriteFile(fullpath, []byte(data), 0700)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

if errors.Is(err, syscall.EINTR) {
continue
}
return err

copybara-service bot pushed a commit that referenced this pull request Jul 28, 2020
After deploying a version of gvisor built with Go 1.14, we're seeing errors setting up cgroups (we manually run `runsc` via `runsc run`, which creates the cgroup). This turns out to be a known issue with Go: golang/go#38033. Given that the [fix won't be backported](golang/go#39026 (comment)), we should retry writes that may fail with EINTR.

This is also what runc does: opencontainers/runc#2258

FUTURE_COPYBARA_INTEGRATE_REVIEW=#3102 from stripe:andrew/cgroup-eintr 079123b
PiperOrigin-RevId: 323575152
@copybara-service copybara-service bot merged commit 8518800 into google:master Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA has been signed ready to pull
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants