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 2021/02/11 08:12:27 UTC

[GitHub] [james-project] chibenwa opened a new pull request #291: JAMES-3500 Fasten tests with reuseForks

chibenwa opened a new pull request #291:
URL: https://github.com/apache/james-project/pull/291


   ```
   07:41:22,949 [INFO] Apache James :: Server :: Web Admin server integration tests :: Distributed SUCCESS [27:23 min]
   07:41:22,945 [INFO] Apache James :: Server :: JMAP (draft) :: RabbitMQ + Object Store + Cassandra Integration testing SUCCESS [25:27 min]
   07:41:22,933 [INFO] Apache James :: Server :: Cassandra with RabbitMQ - guice injection SUCCESS [26:34 min]
   07:41:22,877 [INFO] Apache James :: Mailbox :: Cassandra ............... SUCCESS [24:50 min]
   07:41:22,893 [INFO] Apache James MPT Imap Mailbox - Cassandra .......... SUCCESS [15:46 min]
   07:41:22,947 [INFO] Apache James :: Server :: JMAP RFC-8621 :: Distributed Integration Testing SUCCESS [15:17 min]
   07:41:22,930 [INFO] Apache James :: Server :: Cassandra - guice injection SUCCESS [11:41 min]
   07:41:22,950 [INFO] ------------------------------------------------------------------------
   07:41:22,950 [INFO] BUILD SUCCESS
   07:41:22,951 [INFO] ------------------------------------------------------------------------
   07:41:22,951 [INFO] Total time:  04:37 h
   07:41:22,951 [INFO] Finished at: 2021-02-11T07:41:22Z
   07:41:22,952 [INFO] ------------------------------------------------------------------------
   ```
   
   6 projects takes more than 15 minutes to test.
   
   Looking deeper, they do not use forks. Hence each test class will:
   
     - Initialize testContainers
     - Rebuild the custom Cassandra image
     - Start its own containers (no reuse across forks is possible due to testcontiner lifecycle management)
     - Starts a not warm James, with not yet efficient JIT - slowing down the startup of the first test up to a factor 10.
     
   All of that leads to a time waste of several minutes per test class.
   
   I believe huge gains can be made by leveraging fork reuse - as we did in the past. Please note that I dropped the fork count where relevant - we likely gain way more with fork reuse.


----------------------------------------------------------------
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.

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 change in pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #291:
URL: https://github.com/apache/james-project/pull/291#discussion_r634347878



##########
File path: backends-common/cassandra/src/test/java/org/apache/james/backends/cassandra/DockerCassandra.java
##########
@@ -115,6 +116,18 @@ private void grantPermissionToTestingUser(Cluster privilegedCluster, String keys
         DockerfileBuilder applyStep(DockerfileBuilder builder);
     }
 
+    /**
+     * @return a string to append to image names in order to avoid conflict with concurrent builds
+     */

Review comment:
       I noticed the same. Maybe we should have an env variable dedicated to turn on that "unique C* docker image" on the CI.
   
   What would you think about it @jeantil ?




-- 
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.

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 closed pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
chibenwa closed pull request #291:
URL: https://github.com/apache/james-project/pull/291


   


----------------------------------------------------------------
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.

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 #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #291:
URL: https://github.com/apache/james-project/pull/291#issuecomment-778737854


   Achieved green builds in 3h30.
   
   I'm merging this first batch, and will keep on monitoring build improvments / stability
   
   For the record here is stable test output:
   
   ```
   
   05:01:33,743 [INFO] ------------------------------------------------------------------------
   05:01:33,744 [INFO] Reactor Summary for Apache James Project 3.6.0-SNAPSHOT:
   05:01:33,744 [INFO] 
   05:01:33,772 [INFO] Apache James :: Server :: Blob :: Cassandra ........ SUCCESS [05:04 min]
   05:01:33,773 [INFO] Apache James :: Mailbox :: Cassandra ............... SUCCESS [08:00 min]
   05:01:33,789 [INFO] Apache James MPT Imap Mailbox - Cassandra .......... SUCCESS [05:28 min]
   05:01:33,826 [INFO] Apache James :: Server :: Cassandra - guice injection SUCCESS [09:28 min]
   05:01:33,841 [INFO] Apache James :: Server :: Mailets Integration Testing SUCCESS [15:01 min]
   05:01:33,841 [INFO] Apache James :: Server :: JMAP (draft) :: Memory Integration testing SUCCESS [07:45 min]
   05:01:33,842 [INFO] Apache James :: Server :: JMAP (draft) :: RabbitMQ + Object Store + Cassandra Integration testing SUCCESS [07:06 min]
   05:01:33,843 [INFO] Apache James :: Server :: JMAP RFC-8621 :: Distributed Integration Testing SUCCESS [03:04 min]
   05:01:33,843 [INFO] Apache James :: Server :: JMAP RFC-8621 :: Memory Integration Testing SUCCESS [03:41 min]
   05:01:33,845 [INFO] Apache James :: Server :: Web Admin server integration tests :: Distributed SUCCESS [11:46 min]
   05:01:33,846 [INFO] ------------------------------------------------------------------------
   05:01:33,847 [INFO] BUILD SUCCESS
   05:01:33,847 [INFO] ------------------------------------------------------------------------
   05:01:33,847 [INFO] Total time:  02:56 h
   05:01:33,848 [INFO] Finished at: 2021-02-14T05:01:33Z
   05:01:33,848 [INFO] ------------------------------------------------------------------------
   ```
   
   I will try in another PR to:
   
    - Further tweek the forkCounts where relevant
    - Split mailet-integration tests so that not `reuseFork` friendly MockSmtpServer docker is in its own module.


----------------------------------------------------------------
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.

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] jeantil commented on pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #291:
URL: https://github.com/apache/james-project/pull/291#issuecomment-777416378


   For the context the reuseForks were disabled because of the `OutOfMemoryError: Direct buffer memory` that were frequently encountered when reusing forks and disappeared when forking new processes. 
   
   note that this PR seems to onboard #289 i'm not sure if this is on purpose...


----------------------------------------------------------------
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.

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] jeantil commented on a change in pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #291:
URL: https://github.com/apache/james-project/pull/291#discussion_r634408214



##########
File path: backends-common/cassandra/src/test/java/org/apache/james/backends/cassandra/DockerCassandra.java
##########
@@ -115,6 +116,18 @@ private void grantPermissionToTestingUser(Cluster privilegedCluster, String keys
         DockerfileBuilder applyStep(DockerfileBuilder builder);
     }
 
+    /**
+     * @return a string to append to image names in order to avoid conflict with concurrent builds
+     */

Review comment:
       at the moment you have the opposite behavior where you can set BUILD_ID env variable and it will use a fixed image ... most CI platform I know use the `CI` env variable to let code know it is being run on CI, you could use that I guess. 




-- 
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.

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 #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #291:
URL: https://github.com/apache/james-project/pull/291#issuecomment-778737876


   Merged


----------------------------------------------------------------
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.

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] jeantil commented on pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #291:
URL: https://github.com/apache/james-project/pull/291#issuecomment-777429023


   I had a deeper look and noticed you switched some reuseFork from `1C` to `2` ... 1C would have been a *lot* of forks on the ci platform... I think that's a good change hopefully it will help with the OOME


----------------------------------------------------------------
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.

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 #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #291:
URL: https://github.com/apache/james-project/pull/291#issuecomment-778151497


   
   These changes are beneficials (down 1h)
   
   Here is a list of the top 10 maven projects where time is spent... All of them takes 5 min +...
   
   ```
   11:14:11,899 [INFO] Apache James :: Mailbox :: Cassandra ............... SUCCESS [13:00 min]
   11:14:11,951 [INFO] Apache James :: Server :: Cassandra - guice injection SUCCESS [11:49 min]
   11:14:11,886 [INFO] apache-james-backends-es ........................... SUCCESS [11:43 min]
   11:14:11,970 [INFO] Apache James :: Server :: Web Admin server integration tests :: Distributed FAILURE [09:58 min]
   11:14:11,966 [INFO] Apache James :: Server :: JMAP (draft) :: Memory Integration testing SUCCESS [08:22 min]
   11:14:11,887 [INFO] Apache James RabbitMQ backend ...................... SUCCESS [07:26 min]
   11:14:11,915 [INFO] Apache James MPT Imap Mailbox - Cassandra .......... SUCCESS [06:55 min]
   11:14:11,966 [INFO] Apache James :: Server :: JMAP (draft) :: RabbitMQ + Object Store + Cassandra Integration testing SUCCESS [06:47 min]
   11:14:11,933 [INFO] Apache James :: Third Party :: LinShare ............ SUCCESS [06:22 min]
   11:14:11,965 [INFO] Apache James :: Server :: Mailets Integration Testing SUCCESS [05:37 min]
   11:14:11,971 [INFO] Total time:  03:09 h
   ```
   
   I will however cherry-pick some build stability fixes...
   
   Maybe...
    - We can try Cassandra mailbox + MPT on 2 forks?
    - We need to investigate why backends-common/elasticsearch & backends-common/rabbitmq takes so much time
   


----------------------------------------------------------------
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.

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 #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #291:
URL: https://github.com/apache/james-project/pull/291#issuecomment-778530435


   Latest build failed with
   
   ```
     | Test Result (4 failures  / +4)org.apache.james.mailets.RemoteDeliveryDKIMIntegrationTest$WhenEnable8BitMime.remoteDeliveryCouldBreakDKIMSignWhenTextMessage{String,  String, SMTPMessageSender, DockerMockSmtp}[1]org.apache.james.smtp.dsn.DSNRelayTest.remoteDeliveryShouldCarryOverDSNMailParameters{DockerMockSmtp}org.apache.james.smtp.dsn.DSNRemoteIntegrationTest.givenAMailWithNotifyNeverWhenItSucceedThenNoDsnIsSentBackorg.apache.james.transport.mailets.RemoteDeliveryErrorHandlingTest.reprocessedTemporaryFailuresShouldEventuallySucceed{SMTPMessageSender,  DockerMockSmtp}
   ```
   
   If I hit it a second time I might consider a revert and defaulting to a log waiting strategy (2 build did not encounter these failures)


----------------------------------------------------------------
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.

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] vttranlina commented on a change in pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #291:
URL: https://github.com/apache/james-project/pull/291#discussion_r634284045



##########
File path: backends-common/cassandra/src/test/java/org/apache/james/backends/cassandra/DockerCassandra.java
##########
@@ -115,6 +116,18 @@ private void grantPermissionToTestingUser(Cluster privilegedCluster, String keys
         DockerfileBuilder applyStep(DockerfileBuilder builder);
     }
 
+    /**
+     * @return a string to append to image names in order to avoid conflict with concurrent builds
+     */

Review comment:
       I have a bit curious.
   When concurrent builds?
   Thank you
   
   // The reason for this curiosity is my computer create a new docker image for each run integrate test.  (It takes up quite a bit of memory). So I want only one image.
   Thank you




-- 
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.

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] vttranlina commented on a change in pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #291:
URL: https://github.com/apache/james-project/pull/291#discussion_r634284045



##########
File path: backends-common/cassandra/src/test/java/org/apache/james/backends/cassandra/DockerCassandra.java
##########
@@ -115,6 +116,18 @@ private void grantPermissionToTestingUser(Cluster privilegedCluster, String keys
         DockerfileBuilder applyStep(DockerfileBuilder builder);
     }
 
+    /**
+     * @return a string to append to image names in order to avoid conflict with concurrent builds
+     */

Review comment:
       I have a bit curious.
   When concurrent builds?
   Thank you
   
   // The reason for this curiosity is my computer create a new docker image for each run integrate test.  (It takes up quite a bit of memory). So I want only one image.




-- 
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.

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 #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #291:
URL: https://github.com/apache/james-project/pull/291#issuecomment-777427029


   > note that this PR seems to onboard #289 i'm not sure if this is on purpose...
   
   It is. Cumulating the effects enables better comparison with the target.
   
   > For the context the reuseForks were disabled because of the OutOfMemoryError: Direct buffer memory that were frequently encountered when reusing forks and disappeared when forking new processes.
   
   Let see if they show up ;-)


----------------------------------------------------------------
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.

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] jeantil commented on a change in pull request #291: JAMES-3500 Fasten tests with reuseForks

Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #291:
URL: https://github.com/apache/james-project/pull/291#discussion_r634409930



##########
File path: backends-common/cassandra/src/test/java/org/apache/james/backends/cassandra/DockerCassandra.java
##########
@@ -115,6 +116,18 @@ private void grantPermissionToTestingUser(Cluster privilegedCluster, String keys
         DockerfileBuilder applyStep(DockerfileBuilder builder);
     }
 
+    /**
+     * @return a string to append to image names in order to avoid conflict with concurrent builds
+     */

Review comment:
       the reason the random extension was introduced was that we suspected a reaper process from deleting the images during the build using a random suffix made sure the image wasn't used for too long




-- 
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.

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