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 build does not wait for base image #2855

Closed
gaetansnl opened this issue Jan 28, 2020 · 9 comments
Closed

Parallel build does not wait for base image #2855

gaetansnl opened this issue Jan 28, 2020 · 9 comments
Labels
enhancement New feature or request

Comments

@gaetansnl
Copy link

I'm building an image B and then 5 images based on B for performance reason.
Since parallel builds are enabled, it seems that B is build 3 times instead of one.

An option to specify dependencies for docker_build and custom_build would be great. Or maybe is there is an existing solution ?

Thank you

@dbentley
Copy link
Contributor

We don't support deps from image to image.

We do allow deps from resource to resource, using resource_deps. As a hack, you could make a fake resource (like a Job that just executes true) to allow this until we support image deps.

@maiamcc
Copy link
Contributor

maiamcc commented Feb 3, 2020

hey @gaetansnl, following up: yes, this is great feature request!

I want to clarify that resource dependencies will only really be useful on the first build. (i.e. resource dependencies only enforce that resource A must be ready before resource B builds for the first time; but do NOT enforce that, when resource A rebuilds, resource B must then rebuild as well.)

As a hack, you might consider:

local_resource('build base image',
    cmd='docker build . -t base-image)
docker_build('child-1', '.', dockerfile='dockerfile.1',
    resource_deps='build base image')
docker_build('child-2', '.', dockerfile='dockerfile.2',
    resource_deps='build base image')
...

with this tiltfile, the local resource defined at the top would build your base image at the beginning of your tilt run. nothing here would stop the image from rebuilding 3x on a change to ., but at least you get the benefit of SOME caching (and hopefully you can leverage live update to reduce the number of docker builds).

I don't know when we'll be able to implement good support for base images + parallel updates, so in the meantime you can use a hack like the above or turn off parallel updates (update_settings(max_parallel_updates=1). Let me know if you have any questions!

Note for Tilt Team: the lightest-weight technical solution is probably to add a filter to NextTargetToBuild like RemoveTargetsSharingImageTargets or something, which removes from consideration any targets that have image targets matching a currently building image.

@nicks
Copy link
Member

nicks commented Feb 28, 2020

OK, with the latest Tilt release, the scheduler won't try to parallelize two deployments that have the same base image. But it will still repeat the builds. I'm going to leave this open for the general case of skipping the redundant builds entirely.

@kollender
Copy link

@nicks any updates regarding redundant builds?

@nicks
Copy link
Member

nicks commented Apr 27, 2020

@kollender Nope, we're still scoping out possible solutions. This is currently difficult to solve without a major internal rearchitecture or some clever idea.

@tclasen
Copy link

tclasen commented May 5, 2020

@nicks , the major internal rearchitecture you will need is to change the list of build objects from graph flattened into a list via a topological sort into a tree-like structure where every node in the graph has a reference to the items blocking it.

Your code: https://github.com/windmilleng/tilt/blob/master/pkg/model/target_graph.go#L5

Example implementation from another project: https://github.com/thought-machine/please/blob/dc22858f1e067bdf3f9ce79cea5ac1a2edae63c7/src/core/graph.go#L17

Both projects also use starlark to mark these dependencies, so there should be a lot of overlap in codebases.

@nicks
Copy link
Member

nicks commented Jun 16, 2020

as of #3362, Tilt will no longer build the same image multiple times in this case

But it will still build them in serial. I filed a separate issue for the parallelization: #3468

There's already a separate issue filed to make custom_build support image dependencies: #2856

@nicks nicks closed this as completed Jun 16, 2020
@nicks
Copy link
Member

nicks commented Jun 24, 2020

unfortunately, we had to roll the fix for this back 😭

@nicks nicks reopened this Jun 24, 2020
@nicks
Copy link
Member

nicks commented Jul 7, 2020

rolled forward a fix in #3553 that i feel a lot better about

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

No branches or pull requests

6 participants