You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/04/30 10:38:11 UTC

[GitHub] [beam] echauchot opened a new pull request #11578: [BEAM-8025] Increase the number of retrials en retrial delay in case of load in CassandraIOTest

echauchot opened a new pull request #11578:
URL: https://github.com/apache/beam/pull/11578


   
   
   **Please** add a meaningful description for your change here
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
   --- | --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   


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



[GitHub] [beam] echauchot commented on a change in pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
echauchot commented on a change in pull request #11578:
URL: https://github.com/apache/beam/pull/11578#discussion_r419272092



##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {

Review comment:
       The Exception is thrown by [ControlConnection.reconnectInternal()](https://github.com/datastax/java-driver/blob/a8339723193b68b9f49c469277ceb12b8181a2c8/driver-core/src/main/java/com/datastax/driver/core/ControlConnection.java#L267) in the datastax driver component. It is thrown when the client has tried to connect to all newly created cassandra nodes. I'm not sure that all types of exceptions are suited for retrial upon load hence this single catch. WDYT ?




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



[GitHub] [beam] mxm commented on a change in pull request #11578: [BEAM-8025] Increase the number of retrials en retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11578:
URL: https://github.com/apache/beam/pull/11578#discussion_r418487133



##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;

Review comment:
       ```suggestion
       Exception exception = null;
       int tried = 0;
   ```

##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {

Review comment:
       I couldn't find that this exception of type RuntimeException is thrown during cluster startup. We may want to catch all `Exception` here. 

##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {
         tried++;

Review comment:
       ```suggestion
           if (exception == null) {
             exception = e;
           } else {
             exception.addSuppressed(e);
           }
           tried++;
   ```

##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {
         tried++;
         try {
-          Thread.sleep(1000L);
+          Thread.sleep(delay);
         } catch (InterruptedException e1) {

Review comment:
       ```suggestion
           } catch (InterruptedException e1) {
             Thread.currentThread().interrupt();
             throw new RuntimeException("interrupted");
           }
   ```

##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {
         tried++;
         try {
-          Thread.sleep(1000L);
+          Thread.sleep(delay);
         } catch (InterruptedException e1) {
         }
       }
     }
-    throw new RuntimeException("Unable to create embedded Cassandra cluster");
+    throw new RuntimeException(
+        String.format(
+            "Unable to create embedded Cassandra cluster: tried %d times with %d delay",
+            tried, delay));

Review comment:
       ```suggestion
       throw new RuntimeException(
           String.format(
               "Unable to create embedded Cassandra cluster: tried %d times with %d delay",
                   tried, delay), 
               exception);
   ```




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



[GitHub] [beam] echauchot commented on a change in pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
echauchot commented on a change in pull request #11578:
URL: https://github.com/apache/beam/pull/11578#discussion_r419273629



##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {
         tried++;

Review comment:
       +1 thanks for the suggestion !




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



[GitHub] [beam] echauchot commented on a change in pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
echauchot commented on a change in pull request #11578:
URL: https://github.com/apache/beam/pull/11578#discussion_r419273767



##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {
         tried++;
         try {
-          Thread.sleep(1000L);
+          Thread.sleep(delay);
         } catch (InterruptedException e1) {
         }
       }
     }
-    throw new RuntimeException("Unable to create embedded Cassandra cluster");
+    throw new RuntimeException(
+        String.format(
+            "Unable to create embedded Cassandra cluster: tried %d times with %d delay",
+            tried, delay));

Review comment:
       +1 thanks for the suggestion !




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



[GitHub] [beam] echauchot commented on a change in pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
echauchot commented on a change in pull request #11578:
URL: https://github.com/apache/beam/pull/11578#discussion_r419272092



##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {

Review comment:
       The Exception is thrown by _ControlConnection.reconnectInternal()_ in the datastax driver component. It is thrown when the client has tried to connect to all newly created cassandra nodes. I'm not sure that all types of exceptions are suited for retrial upon load hence this single catch. WDYT ?




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



[GitHub] [beam] echauchot commented on a change in pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
echauchot commented on a change in pull request #11578:
URL: https://github.com/apache/beam/pull/11578#discussion_r419274259



##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {
         tried++;
         try {
-          Thread.sleep(1000L);
+          Thread.sleep(delay);
         } catch (InterruptedException e1) {

Review comment:
       Yeah, that was too quick of me to ignore the interrupted exception :)




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



[GitHub] [beam] mxm commented on a change in pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11578:
URL: https://github.com/apache/beam/pull/11578#discussion_r419302546



##########
File path: sdks/java/io/cassandra/src/test/java/org/apache/beam/sdk/io/cassandra/CassandraIOTest.java
##########
@@ -153,18 +153,22 @@ public static void beforeClass() throws Exception {
 
   private static Cluster buildCluster(CassandraEmbeddedServerBuilder builder) {
     int tried = 0;
-    while (tried < 3) {
+    int delay = 5000;
+    while (tried < 5) {
       try {
         return builder.buildNativeCluster();
       } catch (NoHostAvailableException e) {

Review comment:
       Seems fair.




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



[GitHub] [beam] echauchot commented on pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
echauchot commented on pull request #11578:
URL: https://github.com/apache/beam/pull/11578#issuecomment-623337551


   @mxm addressed your comments, PTAL.


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



[GitHub] [beam] echauchot commented on pull request #11578: [BEAM-8025] Increase the number of retrials and retrial delay in case of load in CassandraIOTest

Posted by GitBox <gi...@apache.org>.
echauchot commented on pull request #11578:
URL: https://github.com/apache/beam/pull/11578#issuecomment-623323079


   Thanks Max for the review and the suggestions !


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