Added margo_set_progress_when_needed

[mdorier]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.

Project coverage is 57.89%. Comparing base (ac07b7d) to head (ff65a22).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/margo-core.c	63.63%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #296      +/-   ##
==========================================
+ Coverage   57.82%   57.89%   +0.07%     
==========================================
  Files          70       70              
  Lines       10183    10198      +15     
  Branches     1338     1341       +3     
==========================================
+ Hits         5888     5904      +16     
  Misses       3458     3458              
+ Partials      837      836       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[carns]

Maybe these macros can be disabled (i.e. no locking etc.) if margo is initialized in listening mode? In that case (on the server side, when unexpected requests could arrive at any time) we always have to call progress, but on clients you only need to call progress when there is margo activity.

[mdorier]

It's disabled by default unless you call margo_set_progress_when_needed(mid, true). I don't want to disable it automatically when the mid is listening, because for instance in Mofka the client has to be listening: it sends RPCs to the server and within these RPCs, the server sends RPCs back to the client. In this situation the fact that the client has an on-going RPC makes it such that the progress loop is unblocked and ready to receive the RPC from the server.

EDIT: if anything, we could make "true" the default is the mid is NOT listening.

[mdorier]

Experiments with Mofka shows that it works as intended. @carns any objection to merging it? Should I make this behavior the default for margo instances initialized with listening = false? (technically I think you could have situation where things break, e.g. the client sends an RPC with a bulk handle to the server, the server responds, and keeps the bulk handle around to make RDMA operations later, outside of the RPC, and you're using a tcp transport or any transport where RDMA is emulated and requires explicit progress).

[carns]

Oh, I see. I overlooked the logic was opt-in. Also I didn't realize that mofka clients were receiving RPCs, that's interesting.

At any rate, I'm fine with this being merged, but my preference would be for it to remain opt in for now. Possibly automatically enabled later but we should take our time thinking about the use cases.

[mdorier]

Ok, merging.