search

eclipse-apoapsis / ort-server

A scalable server implementation of the OSS Review Toolkit.
Apache License 2.0
10 stars 3 forks source link

Simplify sender and receiver configs #101

Open mmurto opened 2 weeks ago

mmurto commented 2 weeks ago

Core configures orchestrator queue that it sends to as:

https://github.com/eclipse-apoapsis/ort-server/blob/9c96a53f934e995c79638c58e26b088a2cb8fc47/core/src/main/resources/application.conf#L96-L109

and orchestrator configures the queue that it receives from as:

https://github.com/eclipse-apoapsis/ort-server/blob/9c96a53f934e995c79638c58e26b088a2cb8fc47/orchestrator/src/main/resources/application.conf#L56-L69

Is there ever a situation where ORCHESTRATOR_SENDER_TRANSPORT_* would be different from ORCHESTRATOR_RECEIVER_TRANSPORT_*? If not, it would be clearer if they both would use ORCHESTRATOR_TRANSPORT_* or something like that. Same applies to other senders and receivers as well.

oheger-bosch commented 2 weeks ago

There can be indeed asymmetry between senders and receivers. In our setup, Orchestrator sends messages to the workers via the Kubernetes transport, but the workers respond via a message queue. But I think you are right wrt Orchestrator: as the central component that receives messages from all others, the receiver protocol always corresponds to the sender protocols.

I am not sure however, whether your proposed changes bring a significant simplification. The transport protocol still needs to be declared in the configuration files for all components. The configuration is separated into a sender and receiver configuration which is necessary because of the mentioned asymmetry.

mmurto commented 2 weeks ago

There can be indeed asymmetry between senders and receivers. In our setup, Orchestrator sends messages to the workers via the Kubernetes transport, but the workers respond via a message queue. But I think you are right wrt Orchestrator: as the central component that receives messages from all others, the receiver protocol always corresponds to the sender protocols.

I am not sure however, whether your proposed changes bring a significant simplification. The transport protocol still needs to be declared in the configuration files for all components. The configuration is separated into a sender and receiver configuration which is necessary because of the mentioned asymmetry.

Thanks for the clarification! Maybe the actual thing that confuses me a little is the naming based on the receiver and sender instead of the actual queue. Orchestrator Sender and Receiver are both actually the orchestrator_queue, and same applies to all other senders and receivers. And IIUC, there will always be a matching sender and receiver, or could there be a situation where Core's ORCHESTRATOR_SENDER_TRANSPORT_TYPE is kubernetes and Orchestrator's ORCHESTRATOR_RECEIVER_TRANSPORT_TYPE is rabbitMQ?

Using variable naming based on the actual queue and not the service sending or receiving would clarify this relation and dependency between the two. Similar logic is used with database environment: there's DB_HOST on all services, no ORCHESTRATOR_DB_HOST etc. So maybe instead of the ORCHESTRATOR_TRANSPORT_TYPE etc. I suggested in the first message, there could be ORCHESTRATOR_QUEUE_TYPE and others.

oheger-bosch commented 2 weeks ago

I agree that this approach would simplify the configuration. So, basically only receivers would be configured - according to the transport the affected component is listening. The sender configuration would then be derived from this.

I only fear that this would require a major change in the code that sets up the communication between the workers. Also, in some cases, there are differences in the sender and receiver configuration. For instance, the Kubernetes sender has a rather complex configuration that supports labels, volume mounts and much more, while the receiver is quite simple.