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

Add a flag to Run command to wait for all output before exiting #2478

Open
pwalski opened this issue Feb 28, 2023 · 7 comments
Open

Add a flag to Run command to wait for all output before exiting #2478

pwalski opened this issue Feb 28, 2023 · 7 comments
Assignees
Labels
deferred sdk-request issue requested by SDK team

Comments

@pwalski
Copy link
Contributor

pwalski commented Feb 28, 2023

What:
Add a flag to Run command to wait for all output so that requestors that are prepared to wait can signal it and have guarantees to receive all output produced by the task.

golemfactory/ya-runtime-vm#150 Contains the revert of the waiting mechanism that needs to be adjusted to be conditional on the presence of the flag.

While fixing #2035 it was apparent that vm runtime is not waiting for all output to be sent to exe-unit before sending the process exited notification and exe-unit does not query remaining output after this notification was received.
Adding this wait unconditionally had some unexpected consequences and broke some examples which made it apparent that this is not a backward compatible fix.

@pwalski pwalski added the enhancement New feature or request label Feb 28, 2023
@pwalski
Copy link
Contributor Author

pwalski commented Mar 1, 2023

Could be done together with #1840

@golmek
Copy link
Contributor

golmek commented Apr 4, 2023

result of SDK ticket: golemfactory/yapapi#1091

@golmek
Copy link
Contributor

golmek commented Apr 5, 2023

It has workaround: output can be directed to the file and downloaded

@grisha87
Copy link
Contributor

@prekucki , we would like to give this one a priority, as the "large stdout output" issue is pending resolution on our end and we need this fixed in yagna.

@grisha87
Copy link
Contributor

grisha87 commented Feb 7, 2024

After conversation with @prekucki , we established that this issue will be tackled as part of the project holistically approaching VM implementation.

@grisha87
Copy link
Contributor

grisha87 commented Feb 22, 2024

Note: Some teams stumbled on this issue during the hackathon. Since I was on the spot I've provided them with a workaround. @prekucki, @golmek I don't think we can rely on our users knowing these workarounds. Technology should work in the first place. "Workarounds" can work in business projects where you can train a group of employees to deal with it systemically. This attitude is not going to work with developers using Golem. I wish to remove the "deferred" label and schedule a fix for this.

Additional info from the call with Rekuc pointing one additional area where the problem can appear:

  • cat file.txt <- There's a limited buffer size to prevent the explosion of the memory, we would have to document that in the SDK Result contents can't be bigger than "X" at once.

@grisha87
Copy link
Contributor

@prekucki , can this be the reason for such a behavior: I'm running a command on the provider, the command completes and the stdout is empty? I'm running the same task on multiple providers on the network and in some cases, I get a positive result of the execution but the stdout is missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deferred sdk-request issue requested by SDK team
Projects
None yet
Development

No branches or pull requests

5 participants