Export app image and cache image in parallel #1247
Conversation
Signed-off-by: kritka sahni <[email protected]>
|
Issue #1167 |
|
Spec PR buildpacks/spec#380 |
| @@ -319,9 +319,10 @@ func testExporterFunc(platformAPI string) func(t *testing.T, when spec.G, it spe | |||
| h.WithArgs(exportArgs...), | |||
| ) | |||
| h.AssertStringContains(t, output, "Saving "+exportedImageName) | |||
|
|
|||
| // To detect whether the export of cacheImage and exportedImage is successful | |||
There was a problem hiding this comment.
Should we enable parallel export in this test?
There was a problem hiding this comment.
Taking a look on how to enable that
There was a problem hiding this comment.
Seems to be passing "PASS: TestExporter/acceptance-exporter/0.10/registry_case/first_build/cache/cache_image_case/is_created_with_parallel_export_enabled" -> is this this test enough @natalieparellano ?
There was a problem hiding this comment.
I think latest test-windows is failing due to an unrelated error. It should ideally succeed on retry - I don't have option to retry workflow @natalieparellano . I have added test and fixed warning message for -parallel export option. I will wait for end user testing results to see if we need to make more changes.
| return Layer{}, err | ||
| } | ||
| defer func() { | ||
| if closeErr := lw.Close(); err == nil { |
There was a problem hiding this comment.
Isn't err always nil here?
|
This looks good @kritkasahni-google. Thank you for tackling it in all its complexity. I'll report back with the results of some end-user testing. |
Co-authored-by: Natalie Arellano <[email protected]> Signed-off-by: Kritka Sahni <[email protected]>
@natalieparellano Looking forward to test results. I can jump on fixing any issues as soon as you let me know. Thanks for testing! |
Signed-off-by: kritka sahni <[email protected]>
Signed-off-by: kritka sahni <[email protected]>
Signed-off-by: kritka sahni <[email protected]>
Signed-off-by: kritka sahni <[email protected]>
|
I think latest test-windows is failing due to unrelated error -> |
There was a problem hiding this comment.
Hi @kritkasahni-google - thanks for this. I'm attaching the results of my user-testing - everything works as expected (including cancel, which terminates both export and cache). I tested with the Java/Maven app from the Paketo samples repo. Switching between parallel/not parallel either gives you 1 second faster total build, or 1 second faster access to the app image (while waiting for cache), which makes sense.
@jabrown85 did you want to look at this one again?
| | 1st build (average of 3) | 2nd build (average of 3) | Sample log output |
| --------------- | ------------------------ | ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| Parallel export | 3.99 | 2.95 | Timer: Exporter started at 2023-12-12T16:07:41Z<br>Timer: Exporter ran for 3.45521449s and ended at 2023-12-12T16:07:44Z<br>Timer: Cache started at 2023-12-12T16:07:41Z<br>Timer: Cache ran for 3.526350332s and ended at 2023-12-12T16:07:44Z |
| Parallel cache | 3.72 | 2.55 | |
| Time waiting | 3.99 | 2.95 | |
| | | | |
| Seq export | 2.60 | 2.27 | Timer: Exporter started at 2023-12-12T15:28:34Z<br>Timer: Exporter ran for 2.538096476s and ended at 2023-12-12T15:28:37Z<br>Timer: Cache started at 2023-12-12T15:28:37Z<br>Timer: Cache ran for 2.818896323s and ended at 2023-12-12T15:28:40Z |
| Seq cache | 2.41 | 1.98 | |
| Time waiting | 5.02 | 4.25 |
Thanks @natalieparellano for verifying. @jabrown85 lmk us know if the changes make sense? |
Summary
Export cache image and app image in parallel when parallel-export is enabled by platform.
Release notes
Export cache image and app image in parallel when parallel-export is enabled by platform.
Related
Resolves # 1167
Context
Export cache image and app image in parallel when parallel-export is enabled by platform.
cc - credits Woa [email protected] for initiating this work