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

- parallel StartMojo #531

Closed

Conversation

danielwegener
Copy link
Contributor

@danielwegener danielwegener commented Jul 31, 2016

that respects dependencies and starts all containers in parallel as soon as all dependencies are resolved. This can greatly improve startup time if the waits that you use are mostly io- or sleep related.

My current use-case: Starting 5-8 different applications on tomcats and wait until each of them is fully operational and ready for e2e tests (including hazelcast cluster convergence and JPA h2 initialization).

This feature reduced my regular startup-time from ~5:00 minutes to 1:23.

This is a POC that adds an ExecutionService and ExecutorCompletionService to the StartMojo. The nonParallel configuration simply falls back to a guava SameThreadExecutionService.

The Parallel start is configurable (docker.startParallel, disabled by default).

This PR adds another imageRunConfiguration property dependsOn (naming from docker-compose) which is the link-equivalent to express inter container dependencies in docker custom networks (it delays the start of a container until all of its dependent containers are started (inclusive wait)).

There might be some concurrency bugs lurking around (I saw that most of the DockerAccess state is synchronized). Especially the ordered shutdown is a bit tricky (especially it would be great if we could shutdown the executor by not starting new tasks but also not stopping tasks that are running).

I am open for feedback :)

@rhuss
Copy link
Collaborator

rhuss commented Aug 2, 2016

Thanks a lot ! I will have a look soon. I'm only afraid that when using the Unix socket connection to Docker that we will soon run in the concurrency issues we already have when waiting on logoutput. See #259 for details. If we would find either a unix socket Java lib which is thread safe or we could serialize the access to the socket, this would be perfect.

I had no chance yet to look into the PR, but hope I can do it this week (sorry, still quite busy with other stuff). Maybe you could make some tests with using the Unix socket in the meantime, too ?

@danielwegener
Copy link
Contributor Author

@rhuss Unfortunately I do not have a native docker host setup ready (usually using docker for mac native/machine), but I'll try to set it up later.

@rhuss
Copy link
Collaborator

rhuss commented Aug 3, 2016

I just had the chance for a quick review, looks great ! And even if there are issues with the Unix socket (and soon to come, Windos named pipes, too), we could document that parallel execution is only supported for TCP connections.

Some things still which I'd need:

BTW, I wonder whether this would make sense for building images, too ? well, probably not so much as one does not has many images to build anyway.

Thanks again, will add your contribution in the overnext release (since I have to cut a release right now).

@danielwegener
Copy link
Contributor Author

Thanks for the feedback. All right, gonna try to come up with some tests and docs.

I think I'll put the executorService.awaitTermination timeout to infinity such that, given enough time, all starting containers will eventually start and, given one of the containers fail to start, all started containers will be shut down properly (this assumes that each single start job has its own timeout and will terminate eventually). WDYT?

Concerning image builds: It definitely depends on the build job if the parallel image builds bring real benefits (for my use case its not such a big deal because we just stack some local files onto an already existing base image) - but images with a lot of latency bound io (lots of small network connections, apt-get update for example) could benefit. But I think the effect will not be as strong as waiting for polling start wait conditions.

…ners in parallel as soon as all dependencies are resolved.
@rhuss
Copy link
Collaborator

rhuss commented Aug 10, 2016

Just want to let you know that I will soon be off for PTO in the next three weeks and wont be able to continue here. but after this we should integrated this nice PR.

@danielwegener
Copy link
Contributor Author

Sorry, that took a while (I was also on PTO :)). I tuned the custom-net example to use dependsOn and parallelStart and added some bits of documentation (required some restructuring).

The refactoring/testing of the parallel start is not so easy without either introducing methods which basically "give me the whole world"-parameter lists or by working around the mojo-cdi pattern. We could simply rip out some parts of the startInternal method via extract method - but I am not sure if that would make it much clearer (some parts like the start order resolver could be better be tested in isolation though).

Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the docs & changes, looks good to me.

I'm going to make a release this evening with your changes.

thanks ;-)

@danielwegener
Copy link
Contributor Author

Cool - thanks alot :)

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

Successfully merging this pull request may close these issues.

2 participants