Azure / azure-functions-host

The host/runtime that powers Azure Functions
https://functions.azure.com
MIT License
1.94k stars 442 forks source link

RpcFunctionInvocationDispatcher should have a single responsibility #7598

Open anandagopal6 opened 3 years ago

anandagopal6 commented 3 years ago

RpcFunctionInvocationDispatcher.cs currently initiates the GRPC channel initialization process. This service should ideally focus on invoking functions rather than starting up the communication channels. To support the single responsibility principle, the channel initialization logic should be moved out into a separate service that is called first before calling on the invocation dispatcher to set up invocation buffers and invoke functions.

To understand the current multiple responsibilities, first take a look at RpcFunctionInvocationDispatcher.InitializeAsync. This function solely deals with obtaining existing GRPC channels and starting up new ones. It references many other helper methods such as StartWorkerProcesses and InitializeJobhostLanguageWorkerChannelAsync. These types of channel initialization functions should ideally be stored in a separate channel manager service that can simply be called upon by the invocation dispatcher.


Channel Initialization Helper Functions in RpcFunctionInvocationDispatcher.cs

image


Once this change is made, the worker indexing pipeline functions GetWorkerMetadata and part of FinishInitialization can also move over to this new service because they rely only on GRPC worker channels, not on the invocation dispatcher's true responsibility.

Another main piece of logic that needs to be moved over to the new service is sending function load requests. The invocation dispatcher should not be responsible for this, but the accompanying logic of setting up invocation buffers can stay inside of the invocation dispatcher.

fabiocav commented 3 years ago

Assigning this to 114 for initial design.

fabiocav commented 2 years ago

Pending update from @soninaren , moving to sprint 116