server: always handle streaming RPCs in separate goroutines to optimize serverWorker availability

[pingkuan]

Streaming RPCs are typically long-lived and can monopolize the serverWorker for extended periods. To prevent this, streaming RPCs should always be handled in a separate goroutine.

[linux-foundation-easycla[bot]]

• :white_check_mark:login: pingkuan / (c80387aa545ee799883125dc15d0a8bae6da959c)

The committers listed above are authorized under a signed CLA.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.

Project coverage is 81.77%. Comparing base (cfd14ba) to head (c80387a). Report is 29 commits behind head on master.

Files with missing lines	Patch %	Lines
server.go	76.92%	2 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7558      +/-   ##
==========================================
- Coverage   81.92%   81.77%   -0.16%     
==========================================
  Files         361      361              
  Lines       27816    27828      +12     
==========================================
- Hits        22789    22756      -33     
- Misses       3838     3869      +31     
- Partials     1189     1203      +14     

Files with missing lines	Coverage Δ
server.go	82.04% <76.92%> (-0.19%)	:arrow_down:

... and 24 files with indirect coverage changes

[purnesh42H]

@pingkuan thanks for suggestion. I have asked the team for their thoughts on this change

[purnesh42H]

@pingkuan our suggestion would be to use a signal which identifies long live rpcs instead of restricting to streaming RPCs.

[pingkuan]

@purnesh42H Since the package may not be able to determine in advance whether a method will be long-lived, which is typically a decision made by the user during implementation, I suggest offering a server option that allows users to specify which methods should be treated as long-lived and, consequently, should not be handled by the server worker.

[easwars]

Is there an issue open for this? I feel that it would make sense to open an issue, discuss the options we have, finalize one, and then move on to a PR.

[dfawley]

Is there an issue open for this? I feel that it would make sense to open an issue, discuss the options we have, finalize one, and then move on to a PR.

+1. E.g. I wonder if we can use a sync.Pool to facilitate worker goroutine re-use?