chibenwa commented on code in PR #1403:
URL: https://github.com/apache/james-project/pull/1403#discussion_r1081137417
##########
server/apps/spring-app/src/main/resources/mailetcontainer.xml:
##########
@@ -354,6 +354,11 @@ Regards, Postmaster XXX.YYY
<gateway>otherserver.mydomain.com</gateway>
<gatewayPort>25</gatewayPort>
-->
+ <!-- in case of multiple gateway round-robin load balancing can bi
activated -->
+ <!-- with the loadBalancing flag, default is false -->
+ <!--
+ <loadBalancing>true</loadBalancing>
Review Comment:
Personnally I do not think this setting is necessary to be added.
That is not an objection on my side, but naturrally I would have been
applying it all the time.
##########
server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remote/delivery/RemoteDeliveryConfiguration.java:
##########
@@ -281,6 +288,14 @@ public Collection<String> getGatewayServer() {
return gatewayServer;
}
+ public synchronized Collection<String> getGatewayServerRotateList() {
+ if (!gatewayServer.isEmpty() && gatewayServer.size() > 0) {
+ Collections.rotate(gatewayServerRotateList,1);
+ return List.copyOf(gatewayServerRotateList);
+ }
+ return List.copyOf(gatewayServerRotateList);
+ }
Review Comment:
Here we have a design problem.
-> a Configuration is meant as "immutable" and represent the content of the
conf file.
It should be absent of application logic and shouldn't hold state.
The use of `synchronized` looks like an uneeded bottleneck to me.
I would suggest just copying the collection of gateways and shuffling it
for each outgoing email
-> no state
-> no synchronization.
Thoughts?
##########
server/apps/spring-app/src/main/resources/mailetcontainer.xml:
##########
@@ -354,6 +354,11 @@ Regards, Postmaster XXX.YYY
<gateway>otherserver.mydomain.com</gateway>
<gatewayPort>25</gatewayPort>
-->
+ <!-- in case of multiple gateway round-robin load balancing can bi
activated -->
+ <!-- with the loadBalancing flag, default is false -->
Review Comment:
```suggestion
<!-- with the loadBalancing flag, default is false -->
```
I think default can be true: this is definitly a desirable behaviour.
##########
server/apps/spring-app/src/main/resources/mailetcontainer.xml:
##########
@@ -354,6 +354,11 @@ Regards, Postmaster XXX.YYY
<gateway>otherserver.mydomain.com</gateway>
<gatewayPort>25</gatewayPort>
-->
+ <!-- in case of multiple gateway round-robin load balancing can bi
activated -->
Review Comment:
```suggestion
<!-- in case of multiple gateway round-robin load balancing can be
activated -->
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
