add more flexible support for long running callback handlers

[andymck]

Linked grpcbox_plugin PR: tsloughter/grpcbox_plugin#15

These changes originate from use cases encountered at helium and in integrating this lib into the helium stack.

NOTE: This PR introduces breaking changes for any existing services. Handlers would be required to be updated. I suspect this may be a blocker for this PR, however I am keen to hear thoughts. If it is a blocker I can recut a separate PR with only the necessary changes ( supporting props & maps in the service definitions )

The changes here have been tested as part of the helium stack and using another erlang grpc client from Bluehouse Technology.

Change Summary

• Dont spawn off streaming RPC handlers and instead handle server side incoming msgs within the stream process. This allows for ( albeit a somewhat subjective opinion ) better structured handlers which can more easily maintain their own state. Obviously it also has the benefit of reducing the overall process count for each stream and removes the overhead in managing the spawned process ( such as handling their various exit signals ) and in exchanging msgs between the stream and spawned handler process.

• Allow application callback handlers to define, initialize & maintain their own state within the context of the grpcbox stream. Thread state to all handler calls.

• Allow info messages to be threaded from stream to application callback handlers.

• Handle PB service names as either maps or props

TODO:

• update interop tests

[andymck]

putting to draft until i get tests regressions addressed

[codecov]

Codecov Report

Merging #52 (ba03e7c) into main (22d7a09) will increase coverage by 0.98%.
The diff coverage is 64.00%.

❗ Current head ba03e7c differs from pull request most recent head 1c5c9fb. Consider uploading reports for the commit 1c5c9fb to get more accurate results

@@            Coverage Diff             @@
##             main      #52      +/-   ##
==========================================
+ Coverage   37.83%   38.82%   +0.98%     
==========================================
  Files          28       28              
  Lines        2080     2102      +22     
==========================================
+ Hits          787      816      +29     
+ Misses       1293     1286       -7     

Impacted Files	Coverage Δ
src/grpcbox_stream.erl	60.10% <46.87%> (-4.45%)	⬇️
src/grpcbox_services_sup.erl	90.66% <83.33%> (-0.89%)	⬇️
src/grpcbox_reflection_service.erl	97.36% <100.00%> (+0.22%)	⬆️
src/grpcbox_client.erl	57.14% <0.00%> (-2.02%)	⬇️
src/grpcbox_client_stream.erl	69.33% <0.00%> (ø)
src/grpcbox_oc_stats_handler.erl	58.33% <0.00%> (+58.33%)	⬆️
src/grpcbox_oc_stats.erl	62.50% <0.00%> (+62.50%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22d7a09...1c5c9fb. Read the comment docs.

[andymck]

@tsloughter I am curious to get your views on this change, mainly in relation to handling msgs in the stream context as opposed to spawning a new process.

As noted above it is a breaking change ( maybe i can resolve that but at this point not sure )....honestly I am not sure where this lib is in its life cycle and its usage in the erlang community. Maybe a breaking change is an absolute blocker, maybe you just dont like this approach or maybe you think it has some legs. Anyway appreciate any thoughts you have on this.

[tsloughter]

Did you run into some issues with shutdown or socket closing that led to your most recent commit?

[andymck]

@tsloughter We were seeing the grpcbox_socket process go down sporadically, and during init it would fail to restart as it hit eaddrinuse. The only scenario in which I can reproduce locally is by killing the process and forcing terminate to be bypassed. If this happens the server will fail to restart and end up breaking the sup restart strategy ( which was default, 1, 5), rending grpcbox effectively down.

I cannot determine at this point why the process might be being killed in the wild. Logs only indicate an enotconn error as being the earliest event which in itself should not be a problem.

I suspect there is more going on, but the blunt fix above gets things up and running again and keeps users connected