testcontainers / testcontainers-java

Testcontainers is a Java library that supports JUnit tests, providing lightweight, throwaway instances of common databases, Selenium web browsers, or anything else that can run in a Docker container.
https://testcontainers.org
MIT License
8.04k stars 1.66k forks source link

Add delay between socket write tries? #758

Open mpapirkovskyy opened 6 years ago

mpapirkovskyy commented 6 years ago

https://github.com/testcontainers/testcontainers-java/blob/609f9a10c3c110be405b9cd96fc340c992b79b3b/core/src/main/java/org/testcontainers/utility/ResourceReaper.java#L117

This loop seem to produce too much logs when container start is delayed for some reason.

kiview commented 6 years ago

Do you mean this log? log.warn("Can not connect to Ryuk at {}:{}", hostIpAddress, ryukPort, e);

mpapirkovskyy commented 6 years ago

@kiview yes.

mfolnovic commented 5 years ago

I'd like to create a PR for this, so... What delay would be acceptable? :)

rnorth commented 5 years ago

That sounds good - thanks @mfolnovic.

How about a 1 second interval? Maybe this would still be quite a lot of logs in the case of massive failure, but ... it's a lot less than it currently would be!

bsideup commented 5 years ago

1 second is too much (usually takes less than 100ms), we should not make the tests slower because of logging.

Maybe we should rate limit the logging only?

mfolnovic commented 5 years ago

Rate limiting logging sounds like a better idea to me, nice! :smiley:

Logging once per second or more frequent?

EDIT: definitely needed :smile:

Job's log exceeded limit of 4194304 bytes.
mfolnovic commented 5 years ago

To add to this, I also think we should limit waiting to not end up in infinite loop.

To give you a bit of context why this is important to me. Every time this goes to infinite loop (in CI), I have to cancel CI job. Then, because of this Gitlab CI bug, containers and networks don't clean up.

Ideally, this loop should wait for some amount of seconds before stopping trying and gracefully fail the test.

What do you think? :)

EDIT: ignore this comment, missed the part that's doing this:

if (!ryukScheduledLatch.await(TestcontainersConfiguration.getInstance().getRyukTimeout(), TimeUnit.SECONDS)) {
    throw new IllegalStateException("Can not connect to Ryuk");
}