Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Mar 30, 2022

This is - I hope - final fix for our image caching. I fought with
it for quite a while and it turned out that I was fighting with
buidkit bug when preparing a multiplatform image.

I think I finally got it under control, the main problem was that
when we prepared multi-platform image, seems like the image that
was prepared "last" was overriding the cache that was prepared
by the other platform build. This cause numerous problems with
investigating it, because sometimes it looked like it worked,
when the "other" platform was faster to build and someties it
did not when it was slower. It almost drove me mad.

It looks like however, that the inline cache works better in
this case (and I got it repetetively working) so finally we might
go back to faster builds on CI and fast pull for ./breeze.

I also opened an issue at buildkit, hoping that they will be able
to fix it:

moby/buildkit#2758


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

This is - I hope - final fix for our image caching. I fought with
it for quite a while and it turned out that I was fighting with
buidkit bug when preparing a multiplatform image.

I think I finally got it under control, the main problem was that
when we prepared multi-platform image, seems like the image that
was prepared "last" was overriding the cache that was prepared
by the other platform build. This cause numerous problems with
investigating it, because sometimes it looked like it worked,
when the "other" platform was faster to build and someties it
did not when it was slower. It almost drove me mad.

It looks like however, that the inline cache works better in
this case (and I got it repetetively working) so finally we might
go back to faster builds on CI and fast pull for ./breeze.

I also opened an issue at buildkit, hoping that they will be able
to fix it:

moby/buildkit#2758
@potiuk
Copy link
Member Author

potiuk commented Mar 30, 2022

I think I have FINALLY got the caching working.

CC: @Bowrna - this might slightly modify the #22344

@potiuk
Copy link
Member Author

potiuk commented Mar 30, 2022

BTW. @mik-laj - this MIGHT MEAN that we can get the --link back - but I want to make it one-thing-at-a-time.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Mar 30, 2022
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@potiuk potiuk merged commit 9e66d14 into apache:main Mar 30, 2022
@potiuk potiuk deleted the final-fix-for-caching-images branch March 30, 2022 13:41
@Bowrna Bowrna mentioned this pull request Mar 31, 2022
@potiuk
Copy link
Member Author

potiuk commented Apr 4, 2022

FYI: The problem with caching multi-node builds has been confirmed and it looks like it's a buildx plugin bug,

I closed the buildkit one and opened docker/buildx#1044

@potiuk potiuk restored the final-fix-for-caching-images branch April 26, 2022 20:53
@potiuk potiuk deleted the final-fix-for-caching-images branch July 29, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants