Skip to content

Conversation

sluongng
Copy link
Contributor

This was splitted from #9978 to review separately.

Currently, our executor keeps remote action stdout/stderr inside
in-memory buffers. This approach can cause OOM-kill when the action
stdout/stderr are large.

Introduce a new flag to control the size of stdouterr of remote actions.
Also coupled with it is a new writer struct which returns a
ResourceExhaustedError when the limit is reached.

In a subsequent PR, we shall use this writer to modify different
isolation implementations to control the memory usage of Executor.

This was splitted from #9978 to review separately.

Currently, our executor keeps remote action stdout/stderr inside
in-memory buffers. This approach can cause OOM-kill when the action
stdout/stderr are large.

Introduce a new flag to control the size of stdouterr of remote actions.
Also coupled with it is a new writer struct which returns a
ResourceExhaustedError when the limit is reached.

In a subsequent PR, we shall use this writer to modify different
isolation implementations to control the memory usage of Executor.
Copy link
Member

@bduffany bduffany left a comment

Choose a reason for hiding this comment

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

nice

return &limitWriter{w: w, limit: *StdOutErrMaxSize}
}

// limitWriter limits the number of bytes written to it.
Copy link
Member

Choose a reason for hiding this comment

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

maybe add:

It returns a ResourceExhausted error if a write occurs that would exceed the limit. Before returning a ResourceExhausted error, it writes as many bytes as possible before the limit would be reached.

n uint64
}

func (lw *limitWriter) Write(p []byte) (int, error) {
Copy link
Member

@bduffany bduffany Sep 30, 2025

Choose a reason for hiding this comment

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

I think this can be simplified a bit


func (lw *limitWriter) Write(p []byte) (n int, err error) {
	capacity := min(lw.limit-lw.n, len(p))
	if capacity > 0 {
		n, err = lw.w.Write(p[:capacity])
		lw.n += n
	}
	

	// If we've reached the limit, return an error,
	// but don't mask the Write error if one occurred.
	if err == nil && capacity < len(p) {
		return n, status.ResourceExhaustedErrorf("stdout/stderr size exceeded limit (%d bytes)", lw.limit)
	}

	return n, err
}

do tests pass with this implementation?

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 this pull request may close these issues.

2 participants