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

exec: refactor command execution #486

Merged
merged 3 commits into from
Jan 5, 2024
Merged

exec: refactor command execution #486

merged 3 commits into from
Jan 5, 2024

Conversation

fho
Copy link
Collaborator

@fho fho commented Jan 3, 2024

  • When a command is executed, all it stdout and stderr output is stored in a buffer in memory. The buffer is used when evaluating the command output and to print it if the command execution should fail. This can be very memory intensive when executed tasks generate a lot of output. To reduce the memory footprint: - only up to 16KB of the beginning and up to 16KB of the end of the output is stored per stdout and stderr for the eventual error message if the command execution fails. The missing part is marked in the error message. This should be enough to understand failures and their context. This is implemented with a modified copy of the prefisuffixsaver from the os/exec stdlib package. - The Run() function does not store and return the whole output of a command on success anymore. Run() is used to execute the commands of baur tasks. This will reduce the memory usage. - The new RunCombinedOut() stores the stdout output in a buffer. This function is used to run baur internal commands that only generate few output and the output is required for evaluation.

  • running commands can now be canceled via a passed context. This is not used by anything yet. It will be used to specify timeouts for "baur run".

  • Cmd now supports to specify an additional io.Writer to stream the stdout and stderr output of a command. This enables efficient processing of larger outputs and will be used in a following Pull-Request, to run "git ls-files" to obtain input file digests.

  • when the command output is streamed to the terminal (baur run --show-task-output), the stderr output is now colored red, that makes it distinguishable from stdout output

  • when --show-task-output is not passed to baur run and the execution of a command fails, the stderr and stdout output will now be shown after each other instead of intermixed.

Closes #328

@fho fho self-assigned this Jan 3, 2024
fho added 2 commits January 3, 2024 14:41
When a go program hangs forever, the go runtime detects it and terminates it
with the error message:
	fatal error: all goroutines are asleep - deadlock!

This happened in the TestProcessTerminatesWithParent, where a child process was
only running `select {}` to wait forever until it gets killed.

Replace select{} with a very long time.Sleep() call in the
TestProcessTerminatesWithParent() to prevent that the testcase fails because the
child process gets terminated because of the described reason.
- When a command is executed, all it's stdout and stderr output is stored in a
  buffer in memory. The buffer is used when evaluating the command output and to
  print it if the command execution should fail.
  This can be very memory intensive when executed tasks generate a lot of
  output. To reduce the memory usage:
    - only store up to 16KB of the beginning and up to 16KB of the end of the output
      per stdout and stderr for the eventual error message when the
      command execution fails. The truncated part is marked in the error message.
      This should be sufficient context to understand failures.
      This is implemented with a modified copy of the prefisuffixsaver from the
      os/exec stdlib package.
    - The Run() function does not store and return the whole output of a command
      on success anymore. Run() is used to execute the commands of baur tasks.
      This will reduce the memory usage.
    - The new RunCombinedOut() stores the stdout output in a buffer. This
      function is used to run baur internal commands that only generate few
      output and the output is required for evaluation.

- running commands can now be canceled via a passed context. This is not used by
  anything yet. It will be used to specify timeouts for "baur run".
- Cmd now supports to specify an additional io.Writer to stream the stdout and
  stderr output of a command. This enables efficient processing of larger
  outputs and will be used in a following Pull-Request, to run "git ls-files" to
  obtain input file digests.
- when the command output is streamed to the terminal
  (baur run --show-task-output), the stderr output is now colored red, that
  makes it distinguishable from stdout output
- when --show-task-output is not passed to baur run and the execution of a
  command fails, the stderr and stdout output will now be shown after each other
  instead of intermixed. This ensures that output of both file descriptors can
  not become interlaced and result in bogus output.
…sk-output

When "--show-task-output" is passed to "baur run", the command output is now
written to stderr instead of stdout.
Stderr fits better the output is for diagnostic purposes.
@fho fho marked this pull request as ready for review January 3, 2024 14:07
@fho fho requested a review from FranciscoKurpiel January 3, 2024 14:07
Copy link
Contributor

@nocive nocive left a comment

Choose a reason for hiding this comment

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

based on the PR description and a brief glance, lgtm 👍 ⚡

@fho fho merged commit 74e0352 into main Jan 5, 2024
4 checks passed
@fho fho deleted the exec_refac branch January 5, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Refactor exec package
2 participants