Skip to content

cgroup: retry file writes on EINTR errors#3102

Merged
copybara-service[bot] merged 1 commit intogoogle:masterfrom
stripe-archive:andrew/cgroup-eintr
Jul 28, 2020
Merged

cgroup: retry file writes on EINTR errors#3102
copybara-service[bot] merged 1 commit intogoogle:masterfrom
stripe-archive:andrew/cgroup-eintr

Conversation

@adunham-stripe
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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