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

enable non-blocking reads for streaming outputs #4755

Merged
merged 7 commits into from
Feb 8, 2025

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Feb 6, 2025

This fixes an issue with EB5 on our cluster, running installs with --logtostdout hangs after executing /usr/share/lmod/lmod/libexec/lmod python --terse --show-hidden avail in the early stages. However, same installs without --logtostdout go fine. An manually executing that lmod command also works fine.

Issue seems to be with long/slow commands in streaming mode. The reading action on the output hinders the completion of the command. This PR enables read in non-blocking mode to avoid this issue.

Changelog:

  • use same non-blockig mechanism for streaming and QA command runs
  • replace fix reads at 128 bytes with readline: this does not change functionality as 128 bytes is a (longish) line and this improves readability of logs
  • ensure that last piece of output is printed in logs

@lexming lexming added bug fix EasyBuild-5.0-blocker Blocker for EasyBuild 5.0 labels Feb 6, 2025
@lexming lexming added this to the 5.0 milestone Feb 6, 2025
branfosj
branfosj previously approved these changes Feb 6, 2025
@lexming
Copy link
Contributor Author

lexming commented Feb 6, 2025

@branfosj do not merge yet, there is a minor issue to fix. Now the output looks like:

== 2025-02-06 15:35:33,429 run.py:555 DEBUG Obtained more stdout: b''
== 2025-02-06 15:35:33,932 run.py:555 DEBUG Obtained more stdout: b''
== 2025-02-06 15:35:34,032 run.py:555 DEBUG Obtained more stdout: b''
== 2025-02-06 15:35:34,133 run.py:555 DEBUG Obtained more stdout: b'INFO: gcc: numpy/core/src/multiarray/datetime_busdaycal.c\n'
== 2025-02-06 15:35:34,133 run.py:555 DEBUG Obtained more stdout: b''
== 2025-02-06 15:35:34,233 run.py:555 DEBUG Obtained more stdout: b''
== 2025-02-06 15:35:34,334 run.py:555 DEBUG Obtained more stdout: b''

== 2025-02-06 15:35:34,944 run.py:555 DEBUG Obtained more stdout: b'INFO: gcc: numpy/core/src/multiarray/descriptor.c\n'
== 2025-02-06 15:35:34,944 run.py:555 DEBUG Obtained more stdout: b''
== 2025-02-06 15:35:35,044 run.py:555 DEBUG Obtained more stdout: b'INFO: gcc: numpy/core/src/multiarray/dlpack.c\n'
== 2025-02-06 15:35:35,045 run.py:555 DEBUG Obtained more stdout: b''
== 2025-02-06 15:35:35,147 run.py:555 DEBUG Obtained more stdout: b''

@branfosj branfosj dismissed their stale review February 6, 2025 14:41

possible issue

@lexming
Copy link
Contributor Author

lexming commented Feb 6, 2025

@branfosj if all test pas, this is ready. Now the debug logs are properly formatted:

== 2025-02-07 01:41:14,654 run.py:530 DEBUG Captured stdout: INFO: gcc: numpy/core/src/multiarray/datetime_busdaycal.c
== 2025-02-07 01:41:15,756 run.py:530 DEBUG Captured stdout: INFO: gcc: numpy/core/src/multiarray/descriptor.c
== 2025-02-07 01:41:17,359 run.py:530 DEBUG Captured stdout: INFO: gcc: numpy/core/src/multiarray/dlpack.c
== 2025-02-07 01:41:17,660 run.py:530 DEBUG Captured stdout: INFO: gcc: numpy/core/src/multiarray/dtypemeta.c
== 2025-02-07 01:41:18,061 run.py:530 DEBUG Captured stdout: INFO: gcc: numpy/core/src/multiarray/dragon4.c
== 2025-02-07 01:41:19,564 run.py:530 DEBUG Captured stdout: INFO: gcc: numpy/core/src/multiarray/dtype_transfer.c

@lexming
Copy link
Contributor Author

lexming commented Feb 7, 2025

On a side note, this PR also removes the following print message:

print_msg(f"(streaming) output for command '{cmd_str}':")

AFAICT this is a leftover from 4.9 that does not align with the behavior of run_shell_cmd in 5.0. run_shell_cmd in 5.0.x is not streaming anything to output. It reads stdout dynamically in blocks of 128 bytes, but those are gathered into an output variable which is only printed once and in full.

@boegel boegel enabled auto-merge February 8, 2025 08:58
@boegel boegel merged commit 7927622 into easybuilders:5.0.x Feb 8, 2025
39 checks passed
@lexming lexming deleted the non-block-reads branch February 9, 2025 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix EasyBuild-5.0-blocker Blocker for EasyBuild 5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants