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

libsubprocess: increase priority of output check watcher #6302

Closed
chu11 opened this issue Sep 24, 2024 · 2 comments
Closed

libsubprocess: increase priority of output check watcher #6302

chu11 opened this issue Sep 24, 2024 · 2 comments
Assignees

Comments

@chu11
Copy link
Member

chu11 commented Sep 24, 2024

per conversation in #6281, consider raising the priority of the output check watcher. This can ensure that a user has a chance to empty a full buffer before more data is put into the buffer.

@garlick
Copy link
Member

garlick commented Sep 24, 2024

This is just something to try to clean things up before we do real flow control in this direction.

My prototype priority setter is here: 1932ce5

Just call flux_watcher_set_priority (check_watcher, 1);

That should raise the priority from the default of 0 to 1. if that doesn't work try 2 :-) 2 is the max.

It has to be called before the watcher is started.

@chu11
Copy link
Member Author

chu11 commented Sep 25, 2024

it appears to do what we want it to do. some debug printfs

before

remote_output_buffered:434 len data = 128
remote_out_prep_cb:179
remote_output_buffered:434 len data = 128
remote_output_buffered:456 - call output with 128 bytes  ***
remote_out_check_cb:191
remote_out_check_cb:200 - call output with 128 bytes ***
remote_output_buffered:434 len data = 0
remote_out_prep_cb:179
remote_out_check_cb:191
remote_out_check_cb:207 - call output with EOF

asterisks lines show that we call the output callback in the output buffered function and later the check output function

after

remote_output_buffered:434 len data = 128
remote_out_prep_cb:179
remote_out_check_cb:191
remote_out_check_cb:200 - call output with 128 bytes ***
remote_output_buffered:434 len data = 128 XXX
remote_out_prep_cb:179 XXX
remote_out_check_cb:191
remote_out_check_cb:200 - call output with 128 bytes ***
remote_output_buffered:434 len data = 0
remote_out_prep_cb:179
remote_out_check_cb:191
remote_out_check_cb:207 - call output with EOF

asterisks lines show that we call the output callback in the check callback both times. the XXX lines show that output is re-added to the buffer after the check and we've started a new prep iteration.

@chu11 chu11 self-assigned this Sep 25, 2024
chu11 added a commit to chu11/flux-core that referenced this issue Sep 26, 2024
Problem: If a output buffer is full, we issue an emergency call
to the user's output callback to empty the buffer before more
data is put in it.  This is done because we cannot control the
order in which check callbacks are called.  Now that check
callbacks can have priority set, we can ensure output check
callbacks are called first before a check callback that may put
more data in the buffer.

Set the priority of the output check watcher higher than the
default.  Remove the now unnecessary emergency callback to the
user's output callback.

Fixes flux-framework#6302
chu11 added a commit to chu11/flux-core that referenced this issue Sep 27, 2024
Problem: If a output buffer is full, we issue an emergency call
to the user's output callback to empty the buffer before more
data is put in it.  This is done because we cannot control the
order in which check callbacks are called.  Now that check
callbacks can have priority set, we can ensure output check
callbacks are called first before a check callback that may put
more data in the buffer.

Set the priority of the output check watcher higher than the
default.  Remove the now unnecessary emergency callback to the
user's output callback.

Fixes flux-framework#6302
chu11 added a commit to chu11/flux-core that referenced this issue Oct 1, 2024
Problem: If a output buffer is full, we issue an emergency call
to the user's output callback to empty the buffer before more
data is put in it.  This is done because we cannot control the
order in which check callbacks are called.  Now that check
callbacks can have priority set, we can ensure output check
callbacks are called first before a check callback that may put
more data in the buffer.

Set the priority of the output check watcher higher than the
default.  Remove the now unnecessary emergency callback to the
user's output callback.

Fixes flux-framework#6302
@mergify mergify bot closed this as completed in a7f99af Oct 2, 2024
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

No branches or pull requests

2 participants