libsubprocess: reduce remote output prep/check #5932
Conversation
|
Is there a reason remote subprocesses do not use fbuf watchers? We use them for local subprocess I/O. Maybe all this is already solved there? (I didn't look) |
I can't remember why. Is your idea that we use fbuf watchers and have those "drive" the callbacks to the user vs the prep/check we have setup? My only thought is if it's a huge win, since there's a prep/check in the watcher too. Although, I suppose the watcher already has its "is buffer empty" logic taken care of? Lemme try and figure out the "input" side of this as well, b/c that's trickier. Perhaps the truth of why it's non-watcher based is in there. |
Yes. |
|
I was finally able to have the remote input stream (i.e. "write_buffer" in the code) work by enabling prep/check only when needed. There were basically some racy bits with the remote server regarding sending data (i.e. like stdin) before the remote process started and some racy bits with when the remote process has exited. It's possible this is why the prep/check were enabled all the time, just to make this logic simpler. given this info, will continue conversation in #5919 regarding potential paths forward |
|
oh whoops, sorry you reviewed. Given discussion in #5919 I thought we were going to table this approach, go with the |
|
I probably lost track of the discussion, but it would be nice to reduce the high load on the broker due to the current situation. Would this work as an interim mitigation? (guess we could have input from @garlick) |
Yes, I do believe it would work as a short term mitigation. |
|
Let's go ahead and merge then if @garlick approves? |
Problem: A number of indentations in are incorrect, likely due to a search and replace that changed a function name. Fix the invalid indentations. Note that in one case the proper indentation would exceed 80 chars, necessitating special formatting.
Problem: Profiling shows that a significant amount of time can be spent in the prep/check of remote subprocess output. This is even in the case when the output buffer is empty. Enable the remote output prep/check only when the buffer is non-empty.
grondo
force-pushed
the
issue5919_subprocess_remote_output_check
branch
from
531c1f9
to
51f7b8f
Compare
|
I set MWP here. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5932 +/- ##
==========================================
- Coverage 83.32% 83.30% -0.02%
==========================================
Files 515 515
Lines 83351 83356 +5
==========================================
- Hits 69452 69440 -12
- Misses 13899 13916 +17
|
Problem: Profiling shows that a significant amount of time can be spent in the prep/check of remote subprocess output. This is even in the case when the output buffer is empty.
Enable the remote output prep/check only when the buffer is non-empty.
Side note, after trying and flailing on the "input" side of this, I realized that a bunch of code in
libsubprocess's remote code simply assumes the prep/check for input is always running. So that work requires more thought. Posting this so we can atleast get this in for the next release if I can't figure out the input side by then.