Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LogMessageWaitStrategy for DockerComposeContainer #515

Closed
s2929346 opened this issue Dec 11, 2017 · 6 comments
Closed

LogMessageWaitStrategy for DockerComposeContainer #515

s2929346 opened this issue Dec 11, 2017 · 6 comments

Comments

@s2929346
Copy link

Hi,

I am starting some docker containers via a docker-compose.yml file. In code, I use the DockerComposeContainer class as following:

DockerComposeContainer c = new DockerComposeContainer(<path>).withTailChildContainers(true);
c.starting(null);

I would like to use the LogMessageWaitStrategy (https://github.com/testcontainers/testcontainers-java/blob/master/core/src/main/java/org/testcontainers/containers/wait/LogMessageWaitStrategy.java) on this container. I must wait until certain log events occurred before I can start using the containers. The LogMessageWaitStrategy should be perfect for this.

Is this possible? I am having a hard time to figure out how I should do this.

Thanks guys!

@s2929346
Copy link
Author

s2929346 commented Dec 12, 2017

Hi,

Update from my side. I got it working with the following code:

DockerClient client = DockerClientFactory.instance().client();
WaitingConsumer consumer = new WaitingConsumer();

LogUtils.followOutput(client, <id of container>, consumer, OutputFrame.OutputType.STDOUT, OutputFrame.OutputType.STDERR);

try {
    consumer.waitUntil(frame -> frame.getUtf8String().matches(<regex>), 300, TimeUnit.SECONDS, 1);
} catch (TimeoutException e) {
    // todo - handle exception properly
} finally {
    // todo - unregister logger/consumer!
}

The only problem left here is that I should unregister the consumer in the finally clause. If I don't do this, I think I will run out of memory eventually, because WaitingConsumer.frames is basically unbounded. That said, I have no idea how to do this. I could execute DockerClientFactory.instance().client().stop(), but that will probably kill the whole test-containers library.

I could try to initialize my own docker client, instead of using DockerClientFactory.instance().client(). Do you have any other ideas?

Are there plans to integrate the wait strategy-functionality on DockerComposeContainer?

Thanks guys!

@rnorth
Copy link
Member

rnorth commented Dec 12, 2017

Hi @s2929346
As you've seen I'm afraid we don't have the log wait functionality for docker-compose yet. I can't see it being too difficult to add, but it might still not be something we could add quickly due to other work that's ongoing. Sorry.

What you're doing looks fine as a workaround. As you point out WaitingConsumer.frames being unbounded could cause a problem if your logs are that big, so that is a bug that I'd like to address (as this would affect all users, not just people using docker-compose). We could probably drop log frames as they're consumed, which would be significantly more efficient. Will have a look.
Thanks
Richard

@s2929346
Copy link
Author

s2929346 commented Dec 12, 2017

Hi @rnorth,

Thank you for your thoughts here. I'll continue using my current implementation for now then. An in-house support for this would be great though!

With regards to WaitingConsumer and unbounded buffer: I think you can remove a logging frame once it has been evaluated by the predicate. Since the regex is matched against each frame separately, we don't need the frame anymore after predicate evaluation. And now, looking again to the actual code in WaitingConsumer, this is exactly what you are doing in frames.pollLast(100, TimeUnit.MILLISECONDS). So I think there is no real problem after all. My apologies for the confusion.

While writing this down, it made me wonder if a multi-line regex wait on the complete log so far would ever be useful. This is currently not supported, as each frame is checked individually in WaitingConsumer. This would also make the logic above more complex, as you need to keep the frames longer and need to decide when to remove them anyway.

Thanks!

@s2929346
Copy link
Author

s2929346 commented Dec 12, 2017

Right, I remember what I was thinking earlier: It might be useful to support that a WaitingConsumer (and its underlying docker container callback) could be removed/disposed somehow. I think a WaitingConsumer's job becomes obsolete once the waiting predicate has evaluated to true (of the timeout has reached). Now, after the waitUntil returns or throws TimeoutException, the WaitingConsumer is still receiving frames. And this will lead to out of memory errors eventually. There is no way to stop this.

That's basically what I wanted to do in the finally clause in the code snippet above.

Something like LogUtils.removeFollowOutput(docker client, container id, consumer). But then you would probably need the callback instance and not the consumer instance.

@bsideup
Copy link
Member

bsideup commented Apr 6, 2018

Fixed by #600 (to be released very soon)

@bsideup
Copy link
Member

bsideup commented Apr 6, 2018

Also, Duplicates #174. Will close this one.

@bsideup bsideup closed this as completed Apr 6, 2018
rnorth pushed a commit that referenced this issue Dec 24, 2018
Bumps [influxdb-java](https://github.com/influxdata/influxdb-java) from 2.10 to 2.14.
<details>
<summary>Changelog</summary>

*Sourced from [influxdb-java's changelog](https://github.com/influxdata/influxdb-java/blob/master/CHANGELOG.md).*

> ## 2.14 [2018-10-12]
> 
> ### Fixes
> 
> - Fixed chunked query exception handling [Issue #523](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/523)
> - Memory leak in StringBuilder cache for Point.lineprotocol() [Issue #526](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/521)
> 
> ## 2.13 [2018-09-12]
> 
> ### Fixes
> - MessagePack queries: Exception during parsing InfluxDB version [macOS] [PR #487](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/487)
> - The InfluxDBResultMapper is able to handle results with a different time precision [PR #501](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/501)
> - UDP target host address is cached [PR #502](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/502)
> - Error messages from server not parsed correctly when using msgpack [PR #506](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/506)
> - Response body must be closed properly in case of JSON response [PR #514](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/514)
> - Time is serialized not consistently in MsgPack and Json, missing millis and nanos in MsgPack[PR #517](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/517)
> 
> ### Features
> 
> - Support for Basic Authentication [PR #492](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/492)
> - Added possibility to reuse client as a core part of [influxdb-java-reactive](https://github.com/bonitoo-io/influxdb-java-reactive) client [PR #493](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/493)
> - Retry capability for writing of BatchPoints [PR #503](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/503)
> - Added `BiConsumer` with capability to discontinue a streaming query [Issue #515](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/515)
> - Added `onComplete` action that is invoked after successfully end of streaming query [Issue #515](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/515)
> 
> ## 2.12 [2018-07-31]
> 
> ### Fixes
> 
> - Remove code which checks for unsupported influxdb versions [PR #474](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/474)
> - Unpredictable errors when OkHttpClient.Builder instance is reused [PR #478](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/478)
> 
> ### Features
> 
> - Support for MessagePack [PR #471](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/471)
> - Cache version per influxdb instance and reduce ping() calls for every query call [PR #472](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/472)
> - FAQ list for influxdb-java [PR #475](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/475)
> 
> ### Improvements
> 
> - Test: Unit test to ensure tags should be sorted by key in line protocol (to reduce db server overheads) [PR #476](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/476)
> 
> ## 2.11 [2018-07-02]
> 
> ### Features
> 
> - Allow write precision of TimeUnit other than Nanoseconds [PR #321](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/321)
> - Support dynamic measurement name in InfluxDBResultMapper [PR #423](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/423)
> - Debug mode which allows HTTP requests being sent to the database to be logged [PR #450](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/450)
> - Fix problem of connecting to the influx api with URL which does not points to the url root (e.g. localhots:80/influx-api/) [PR #400] (https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/400)
></table> ... (truncated)
</details>
<details>
<summary>Commits</summary>

- [`91d0f09`](influxdata/influxdb-java@91d0f09) [maven-release-plugin] prepare release influxdb-java-2.14
- [`8ffaeb9`](influxdata/influxdb-java@8ffaeb9) Revert "[maven-release-plugin] prepare release influxdb-java-2.14"
- [`2781da2`](influxdata/influxdb-java@2781da2) [maven-release-plugin] prepare release influxdb-java-2.14
- [`19c69ed`](influxdata/influxdb-java@19c69ed) [maven-release-plugin] prepare for next development iteration
- [`c6d7f25`](influxdata/influxdb-java@c6d7f25) [maven-release-plugin] prepare release influxdb-java-2.14
- [`2f4c594`](influxdata/influxdb-java@2f4c594) Merge pull request [#531](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/531) from heshengbang/master
- [`f653e62`](influxdata/influxdb-java@f653e62) Easy to use try-with-resources, add README.md
- [`c7be9b0`](influxdata/influxdb-java@c7be9b0) Easy to use try-with-resources
- [`4590d18`](influxdata/influxdb-java@4590d18) - added automated SNAPSHOT publishing to Maven Central repository
- [`ce65a41`](influxdata/influxdb-java@ce65a41) - added automated SNAPSHOT publishing to Maven Central repository
- Additional commits viewable in [compare view](influxdata/influxdb-java@influxdb-java-2.10...influxdb-java-2.14)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=org.influxdb:influxdb-java&package-manager=gradle&previous-version=2.10&new-version=2.14)](https://dependabot.com/compatibility-score.html?dependency-name=org.influxdb:influxdb-java&package-manager=gradle&previous-version=2.10&new-version=2.14)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

**Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit.

You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com).

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in the `.dependabot/config.yml` file in this repo:
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants