You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2023/01/19 11:18:26 UTC
[GitHub] [james-project] AdBuch opened a new pull request, #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
AdBuch opened a new pull request, #1403:
URL: https://github.com/apache/james-project/pull/1403
Implement round robin loadbalancing if multiple gateways are defined
Add new Remote Delivery property loadBalancing.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1403:
URL: https://github.com/apache/james-project/pull/1403#issuecomment-1399189163
Nice contribution!
We need to document the new configuration option...
- https://github.com/apache/james-project/blob/master/server/apps/distributed-app/docs/modules/ROOT/partials/RemoteDelivery.adoc
- https://github.com/apache/james-project/blob/5a57af88eeef6bf08a115121d0a1286574663f56/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java#L51 (Java Doc)
Can you add the documentation to this PR?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1403:
URL: https://github.com/apache/james-project/pull/1403#issuecomment-1403721949
Thanks for the contribution!
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] AdBuch commented on a diff in pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by GitBox <gi...@apache.org>.
AdBuch commented on code in PR #1403:
URL: https://github.com/apache/james-project/pull/1403#discussion_r1081233432
##########
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:
yes, but it's kind a standard that it can be optional
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] AdBuch commented on a diff in pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by GitBox <gi...@apache.org>.
AdBuch commented on code in PR #1403:
URL: https://github.com/apache/james-project/pull/1403#discussion_r1081230664
##########
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:
My first solution was with shuffle as well but i preferred a rotation. i switched it back to a shuffle model
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] AdBuch commented on a diff in pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by GitBox <gi...@apache.org>.
AdBuch commented on code in PR #1403:
URL: https://github.com/apache/james-project/pull/1403#discussion_r1081231566
##########
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:
alright its true now
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #1403:
URL: https://github.com/apache/james-project/pull/1403
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by GitBox <gi...@apache.org>.
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] AdBuch commented on pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by "AdBuch (via GitHub)" <gi...@apache.org>.
AdBuch commented on PR #1403:
URL: https://github.com/apache/james-project/pull/1403#issuecomment-1401524002
Documentation is added, is there a way for me to retrigger the pipeline?
You know which tests are failing? I could put some effort in it to make them more stable. or just autorerun them.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1403: JAMES-3876 Load-balancing flag for Remote Delivery Gateways
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1403:
URL: https://github.com/apache/james-project/pull/1403#issuecomment-1402123699
> Documentation is added, is there a way for me to retrigger the pipeline?
For you, no, for me, yes.
> You know which tests are failing? I could put some effort in it to make them more stable. or just autorerun them.
```
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.22.2:test (default-test) on project james-server-webadmin-data: There was a timeout or other error in the fork -> [Help 1]
```
Known. Port binding issue. I retriggered a build.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org