DRILL-7982: Timed out waiting for container port to open #2287

luocooong · 2021-08-05T09:53:06Z

DRILL-7982: Timed out waiting for container port to open

Description

If the default 60s timeout is not sufficient, it can be altered with the withStartupTimeout() method.

See also Waiting for containers to start or be ready

Then, the testcontainers has merged new feature about the For faster startup.

See also Tune Cassandra options for faster startup

Documentation

N/A

Testing

Need the unit tests can run 3 times without the Timed out waiting for XXX.

vdiravka

Hi @luocooong I have already added fix for that n scope of DRILL-7973.
in the following commit.

But since it is not only the one issue related to CI I didn't publish PR yet. I am going to fix CI failures in scope of that ticket.

vdiravka · 2021-08-05T14:00:02Z

...storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/TestCassandraSuit.java

+                        .withEnv("CASSANDRA_SNITCH", "GossipingPropertyFileSnitch") // Tune Cassandra options for faster startup
+                          .withEnv("JVM_OPTS", "-Dcassandra.skip_wait_for_gossip_to_settle=0 -Dcassandra.initial_token=0")
+                            .withEnv("HEAP_NEWSIZE", "128M")
+                              .withEnv("MAX_HEAP_SIZE", "1024M");


We have not much memory on Github CI. Do we need this additional params for our test container? What about Splunk test container then?

@vdiravka
The Cassandra tests seem to keep timing out and breaking the CI. My recollection of the Cassandra plugin was that Cassandra really does take a long time to start up, possibly more than 1 minute. This was why we implemented it to preserve the connection as long as possible rather than creating tons of new connections.

To my knowledge, the Splunk ones are working fine in the CI.

@luocooong Thanks for this. It makes me wonder, should we add the GossipingPropertyFileSnitch parameter to the Cassandra plugin?

The Cassandra tests seem to keep timing out and breaking the CI. My recollection of the Cassandra plugin was that Cassandra really does take a long time to start up, possibly more than 1 minute. This was why we implemented it to preserve the connection as long as possible rather than creating tons of new connections.

I am aware about timing out test of container for Cassandra plugin. I also fixed that several days ago in my branch. But I didn't merge it, because this is not only the one issue on CI and don't fix CI build. For instance the CI build is restarted several times for this PR and still not green.

To my knowledge, the Splunk ones are working fine in the CI.

My question was about memory options only in the above comment and luocooong answered that he is going to eliminate them.

It makes me wonder, should we add the GossipingPropertyFileSnitch parameter to the Cassandra plugin?

GossipingPropertyFileSnitch looks Cassandra specific code only.

luocooong · 2021-08-05T14:38:49Z

@vdiravka Thanks for the comments. I saw that the goal of DRILL-7973 is avoid the OOM (use the direct mem). This pull request is ready to speed up the C* startup. no conflict. I am going to remove the heap options if you already have new ways. (Actually, I think the new version of testcontainers will included these setting for desfult).

vdiravka · 2021-08-05T16:21:14Z

...storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/TestCassandraSuit.java

@@ -65,7 +66,12 @@ public static void tearDownCluster() {

  private static void startCassandra() {
    cassandra = new CassandraContainer<>("cassandra")
-        .withInitScript("queries.cql");
+                      .withInitScript("queries.cql")
+                      .withStartupTimeout(Duration.ofMinutes(3))


It look like 2m is enough. There is no one failure for Cassandra Test Container with this value
vdiravka@b5b3a17

vdiravka

It looks fine. Please consider possible additional improvement in my comments

vdiravka · 2021-08-06T08:36:52Z

...storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/TestCassandraSuit.java

@@ -65,7 +66,10 @@ public static void tearDownCluster() {

  private static void startCassandra() {
    cassandra = new CassandraContainer<>("cassandra")
-        .withInitScript("queries.cql");
+      .withInitScript("queries.cql")
+        .withStartupTimeout(Duration.ofMinutes(2))


Please keep same indent for the methods call chain. For example:

HiveParquetTableMetadataProvider metadataProvider = builder .withEntries(entries) .withHivePartitionHolder(hivePartitionHolder) .withHiveStoragePlugin(hiveStoragePlugin) .withReaderConfig(readerConfig) .withSchema(schema) .build();

drill/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveDrillNativeParquetScan.java

Line 78 in 4aefcef

.withEntries(entries)

vdiravka · 2021-08-06T08:38:14Z

...storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/TestCassandraSuit.java

+      .withInitScript("queries.cql")
+        .withStartupTimeout(Duration.ofMinutes(2))
+          .withEnv("CASSANDRA_SNITCH", "GossipingPropertyFileSnitch") // Tune Cassandra options for faster startup
+            .withEnv("JVM_OPTS", "-Dcassandra.skip_wait_for_gossip_to_settle=0 -Dcassandra.initial_token=0");


Possibly we can do event better here:

-Dcassandra.skip_wait_for_gossip_to_settle=0 -Dcassandra.load_ring_state=false -Dcassandra.initial_token=1 -Dcassandra.num_tokens=nil -Dcassandra.allocate_tokens_for_local_replication_factor=nil

https://github.com/sky-uk/cassandra-operator/issues/103#issuecomment-499040927

cgivre · 2021-08-06T12:43:21Z

@luocooong @vdiravka
Should we merge this? @vdiravka Are you ok with this or does this still conflict with DRILL-7973?

luocooong · 2021-08-06T14:20:44Z

@cgivre Can be merged. Let me try more options (by Vitalii) first. I will use the fastest parameter.

luocooong · 2021-08-07T04:33:46Z

Done. the current options is faster (included the C* startup and tests). Saved 10 seconds.

current options (JDK 8), spend 48 seconds

2021-08-07T03:55:45.7691691Z [INFO] -------------------------------------------------------
2021-08-07T03:55:45.7692311Z [INFO]  T E S T S
2021-08-07T03:55:45.7693143Z [INFO] -------------------------------------------------------

2021-08-07T03:56:33.1223519Z [INFO] Tests run: 30, Failures: 0, Errors: 0, Skipped: 0

current options (JDK 11), spend 40 seconds

2021-08-07T04:00:34.0149031Z [INFO] -------------------------------------------------------
2021-08-07T04:00:34.0149860Z [INFO]  T E S T S
2021-08-07T04:00:34.0150857Z [INFO] -------------------------------------------------------

2021-08-07T04:01:14.6705624Z [INFO] Tests run: 30, Failures: 0, Errors: 0, Skipped: 0

more options (JDK 8), spend 58 seconds

2021-08-07T02:08:46.6741617Z [INFO] -------------------------------------------------------
2021-08-07T02:08:46.6744463Z [INFO]  T E S T S
2021-08-07T02:08:46.6746502Z [INFO] -------------------------------------------------------

2021-08-07T02:09:44.7628630Z [INFO] Tests run: 30, Failures: 0, Errors: 0, Skipped: 0

more options (JDK 11), spend 38 seconds

2021-08-07T02:00:06.0005436Z [INFO] -------------------------------------------------------
2021-08-07T02:00:06.0006095Z [INFO]  T E S T S
2021-08-07T02:00:06.0006901Z [INFO] -------------------------------------------------------

2021-08-07T02:00:44.6733692Z [INFO] Tests run: 30, Failures: 0, Errors: 0, Skipped: 0

vdiravka

It is too fast :) Even .withStartupTimeout(Duration.ofMinutes(2)) is not needed. But let's keep it to avoid CI fails in future.
+1 LGTM

luocooong added the minor-update label Aug 5, 2021

luocooong self-assigned this Aug 5, 2021

vdiravka reviewed Aug 5, 2021

View reviewed changes

luocooong force-pushed the DRILL-7982 branch from db6ef74 to e20f2f4 Compare August 6, 2021 02:35

luocooong marked this pull request as ready for review August 6, 2021 05:24

luocooong requested review from cgivre and vdiravka August 6, 2021 05:24

vdiravka reviewed Aug 6, 2021

View reviewed changes

DRILL-7982: Timed out waiting for container port to open

0423ca0

luocooong force-pushed the DRILL-7982 branch from e20f2f4 to 0423ca0 Compare August 6, 2021 08:48

luocooong requested a review from vdiravka August 6, 2021 08:48

luocooong force-pushed the DRILL-7982 branch from f62e062 to 0423ca0 Compare August 7, 2021 03:05

vdiravka approved these changes Aug 7, 2021

View reviewed changes

cgivre merged commit 3c092b3 into apache:master Aug 8, 2021

luocooong deleted the DRILL-7982 branch August 20, 2021 02:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRILL-7982: Timed out waiting for container port to open #2287

DRILL-7982: Timed out waiting for container port to open #2287

luocooong commented Aug 5, 2021

vdiravka left a comment

vdiravka Aug 5, 2021

cgivre Aug 5, 2021

vdiravka Aug 5, 2021

luocooong commented Aug 5, 2021

vdiravka Aug 5, 2021 •

edited

Loading

vdiravka left a comment

vdiravka Aug 6, 2021

vdiravka Aug 6, 2021

cgivre commented Aug 6, 2021

luocooong commented Aug 6, 2021

luocooong commented Aug 7, 2021

vdiravka left a comment

DRILL-7982: Timed out waiting for container port to open #2287

DRILL-7982: Timed out waiting for container port to open #2287

Conversation

luocooong commented Aug 5, 2021

DRILL-7982: Timed out waiting for container port to open

Description

Documentation

Testing

vdiravka left a comment

Choose a reason for hiding this comment

vdiravka Aug 5, 2021

Choose a reason for hiding this comment

cgivre Aug 5, 2021

Choose a reason for hiding this comment

vdiravka Aug 5, 2021

Choose a reason for hiding this comment

luocooong commented Aug 5, 2021

vdiravka Aug 5, 2021 • edited Loading

Choose a reason for hiding this comment

vdiravka left a comment

Choose a reason for hiding this comment

vdiravka Aug 6, 2021

Choose a reason for hiding this comment

vdiravka Aug 6, 2021

Choose a reason for hiding this comment

cgivre commented Aug 6, 2021

luocooong commented Aug 6, 2021

luocooong commented Aug 7, 2021

vdiravka left a comment

Choose a reason for hiding this comment

vdiravka Aug 5, 2021 •

edited

Loading