-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Implement incremental prebuilds #4167
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
Conversation
/werft run 👍 started the job as gitpod-build-jx-incremental-prebuilds.17 |
4973977
to
32fb208
Compare
Note: The So, in order to facilitate testing, I'm temporarily introducing a |
6220005
to
6996cec
Compare
Works generally well (both full and incremental prebuilds), but my (view incremental prebuilds table)
Opening the specific commit context URL for that prebuild always fails with this error:
Also, when such a pod fails, (view ws-daemon logs)One
and the other says:
Not sure what that's about, but should be investigated if it can be reproduced. EDIT: Here is the complete error message from Google Cloud. Problem seems to be caused by merge conflicts due to an older prebuild:
|
30f4a9d
to
295497f
Compare
components/server/ee/src/prebuilds/start-prebuild-context-parser.ts
Outdated
Show resolved
Hide resolved
TODO:
|
Oh, that reveals a more interesting problem with our init approach. I think we should reset --hard to the context's branch. Also looking at the code for local branch context (i.e. issue context) we don't pass the corresponding remote branch, which is needed now. |
Indeed. I believe what happened was:
The problem is that we intentionally leave "stash pop" conflicts in place when loading a prebuild, delegating the resolution to the user. This no longer makes sense when we load an older prebuild to produce a newer prebuild. I agree we might want to |
Indeed, great catch! Since
|
…base it on origin/HEAD We previously assumed that a prebuild snapshot holds the latest commits, but this changes with incremental prebuilds (where an older prebuild doesn't have the latest commits). Fixes #4167 (comment)
🎰 /werft run 👍 started the job as gitpod-build-jx-incremental-prebuilds.33 |
fb98941
to
211b7d7
Compare
|
211b7d7
to
b3be5bc
Compare
@svenefftinge Which code do you mean specifically? In #4167 (comment) I pointed out: gitpod/components/content-service/pkg/initializer/prebuild.go Lines 101 to 108 in 3607598
|
274e508
to
debcb34
Compare
debcb34
to
acf816e
Compare
acf816e
to
9196637
Compare
Logging a small issue I noticed in testing: Occasionally, the |
Alright, this is now ready for review! 🚀 @svenefftinge @geropl @csweichel please take a look when convenient. 🙏 Notes:
Please ping me if you have any questions! 🌻 |
/werft run 👍 started the job as gitpod-build-jx-incremental-prebuilds.44 |
Works very well using the prebuild and incremental-prebuild context URLs. But I think it is fine to do this later when we'Äve gained more experience with incremental prebuilds. |
9196637
to
cd46302
Compare
Rebased and squashed as hard as I could! 😁 |
This comment has been minimized.
This comment has been minimized.
🎰 /werft run 👍 started the job as gitpod-build-jx-incremental-prebuilds.46 |
🎰 /werft run 👍 started the job as gitpod-build-jx-incremental-prebuilds.47 |
Update: Simple re-deploy resolved the awkward content init / ws-daemon / registry-facade errors from #4167 (comment) 👍 everything works again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me (sans some style things) :)
…ut the correct revision, even for older clones (e.g. incremental prebuild) We previously assumed that a prebuild snapshot holds the latest commits, but this changes with incremental prebuilds (where an older prebuild doesn't have the latest commits, but 'origin' does). Fixes #4167 (comment)
…d after checkout fails, throw them away instead of leaving merge conflicts in the workspace
cd46302
to
da6dee0
Compare
…ut the correct revision, even for older clones (e.g. incremental prebuild) We previously assumed that a prebuild snapshot holds the latest commits, but this changes with incremental prebuilds (where an older prebuild doesn't have the latest commits, but 'origin' does). Fixes #4167 (comment)
…ut the correct revision, even for older clones (e.g. incremental prebuild) We previously assumed that a prebuild snapshot holds the latest commits, but this changes with incremental prebuilds (where an older prebuild doesn't have the latest commits, but 'origin' does). Fixes gitpod-io#4167 (comment)
…ut the correct revision, even for older clones (e.g. incremental prebuild) We previously assumed that a prebuild snapshot holds the latest commits, but this changes with incremental prebuilds (where an older prebuild doesn't have the latest commits, but 'origin' does). Fixes gitpod-io#4167 (comment)
Design doc
https://www.notion.so/gitpod/Incremental-Prebuilds-49c0a840e54348acba05677bdc450841
How to test
Use anio-dev
preview (e.g. https://jx-incremental-prebuilds.staging.gitpod-io-dev.com/workspaces/)Use the
#prebuild/<repo>
URL prefix to trigger a full prebuild for some commit on some repositoryUse
#incremental-prebuild/<repo>
to trigger an incremental prebuild for some (different) commit (this will automatically select a suitable parent prebuild, or fall back to a full prebuild)Verify that both full and incremental prebuilds work as expected (e.g. the project is prebuilt & starts as usual)
Testing with https://github.com/jankeromnes/gitpod-staging-prebuilds may be helpful (the
.gitpod.yml
logs every task run, along with timestamp and workspace ID, and prints a summary -- e.g. 2 different workspace IDs mean "full prebuild", 3 IDs mean "incremental prebuild")Check incremental prebuilds in the DB (e.g. parents, commits, durations, states) like so: