Skip to content
This repository was archived by the owner on Aug 7, 2025. It is now read-only.

Commit a960885

Browse files
William Douglasbryteise
authored andcommitted
Move packs to download synchronously
There are some problems with how we are using multiplexed curl causing huge slowdowns (2-5 times slower). For now rather than rework the multiplexed curl code, switch to using the synchronous code path. Signed-off-by: William Douglas <[email protected]>
1 parent 3338cb2 commit a960885

File tree

9 files changed

+27
-111
lines changed

9 files changed

+27
-111
lines changed

src/swupd_lib/packs.c

Lines changed: 13 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -76,73 +76,24 @@ static int finalize_pack_download(const char *module, int newversion, const char
7676
return err;
7777
}
7878

79-
static void download_free_data(void *data)
80-
{
81-
struct pack_data *pack_data = data;
82-
83-
if (!data) {
84-
return;
85-
}
86-
87-
FREE(pack_data->url);
88-
FREE(pack_data->filename);
89-
FREE(pack_data);
90-
}
91-
92-
static bool download_error(enum download_status status, void *data)
93-
{
94-
struct pack_data *pack_data = data;
95-
96-
if (!data) {
97-
return false;
98-
}
99-
100-
if (status == DOWNLOAD_STATUS_NOT_FOUND) {
101-
102-
enum telemetry_severity level = TELEMETRY_LOW;
103-
104-
// Missing zero packs is a critical problem
105-
if (pack_data->oldversion == 0) {
106-
level = TELEMETRY_CRIT;
107-
}
108-
telemetry(level, "packmissing", "url=%s\n", pack_data->url);
109-
return true;
110-
}
111-
112-
return false;
113-
}
114-
115-
static bool download_successful(void *data)
116-
{
117-
struct pack_data *pack_data = data;
118-
119-
if (!pack_data) {
120-
return false;
121-
}
122-
123-
return finalize_pack_download(pack_data->module, pack_data->newversion, pack_data->filename) == 0;
124-
}
125-
126-
static int download_pack(struct swupd_curl_parallel_handle *download_handle, int oldversion, int newversion, char *module)
79+
static int download_pack(int oldversion, int newversion, char *module)
12780
{
12881
char *url = NULL;
12982
char *filename;
83+
int ret;
13084

13185
filename = statedir_get_delta_pack(module, oldversion, newversion);
13286

133-
struct pack_data *pack_data;
134-
13587
string_or_die(&url, "%s/%i/pack-%s-from-%i.tar", globals.content_url, newversion, module, oldversion);
13688

137-
pack_data = malloc_or_die(sizeof(struct pack_data));
138-
139-
pack_data->url = url;
140-
pack_data->filename = filename;
141-
pack_data->module = module;
142-
pack_data->newversion = newversion;
143-
pack_data->oldversion = oldversion;
89+
ret = swupd_curl_get_file(url, filename);
90+
FREE(url);
91+
if (!ret) {
92+
ret = finalize_pack_download(module, newversion, filename);
93+
}
94+
FREE(filename);
14495

145-
return swupd_curl_parallel_download_enqueue(download_handle, url, filename, NULL, pack_data);
96+
return ret;
14697
}
14798

14899
static double packs_query_total_download_size(struct list *subs, struct manifest *mom)
@@ -224,7 +175,6 @@ int download_subscribed_packs(struct list *subs, struct manifest *mom, bool requ
224175
int err;
225176
int list_length;
226177
int complete = 0;
227-
struct swupd_curl_parallel_handle *download_handle;
228178
char *packs_size;
229179

230180
progress_next_step("download_packs", PROGRESS_BAR);
@@ -263,20 +213,9 @@ int download_subscribed_packs(struct list *subs, struct manifest *mom, bool requ
263213
goto out;
264214
}
265215

266-
/* we need to download some files, so set up curl */
267-
download_handle = swupd_curl_parallel_download_start(get_max_xfer(MAX_XFER));
268-
if (!download_handle) {
269-
list_free_list(need_download);
270-
ret = -1;
271-
goto out;
272-
}
273-
swupd_curl_parallel_download_set_callbacks(download_handle, download_successful, download_error, download_free_data);
274-
275216
/* get size of the packs to download */
276217
download_progress.total_download_size = packs_query_total_download_size(need_download, mom);
277-
if (download_progress.total_download_size > 0) {
278-
swupd_curl_parallel_download_set_progress_callback(download_handle, swupd_progress_callback, &download_progress);
279-
} else {
218+
if (download_progress.total_download_size <= 0) {
280219
debug("Couldn't get the size of the packs to download, using number of packs instead\n");
281220
download_progress.total_download_size = 0;
282221
}
@@ -298,13 +237,12 @@ int download_subscribed_packs(struct list *subs, struct manifest *mom, bool requ
298237
bundle = mom_search_bundle(mom, sub->component);
299238
if (!bundle) {
300239
debug("The manifest for bundle %s was not found in the MoM\n", sub->component);
301-
swupd_curl_parallel_download_cancel(download_handle);
302240

303241
ret = -SWUPD_INVALID_BUNDLE;
304242
goto out;
305243
}
306244

307-
err = download_pack(download_handle, sub->oldversion, sub->version, sub->component);
245+
err = download_pack(sub->oldversion, sub->version, sub->component);
308246

309247
/* fall back for progress reporting when the download size
310248
* could not be determined */
@@ -313,7 +251,6 @@ int download_subscribed_packs(struct list *subs, struct manifest *mom, bool requ
313251
progress_report(complete, list_length);
314252
}
315253
if (err < 0 && required) {
316-
swupd_curl_parallel_download_cancel(download_handle);
317254
ret = err;
318255
goto out;
319256
}
@@ -322,9 +259,10 @@ int download_subscribed_packs(struct list *subs, struct manifest *mom, bool requ
322259
info("Finishing packs extraction...\n");
323260

324261
progress_next_step("extract_packs", PROGRESS_UNDEFINED);
325-
return swupd_curl_parallel_download_end(download_handle, NULL);
262+
return ret;
326263

327264
out:
265+
list_free_list(need_download);
328266
progress_next_step("extract_packs", PROGRESS_UNDEFINED);
329267
return ret;
330268
}

test/functional/bundleadd/add-json.bats

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,8 @@ test_setup() {
3232
{ "type" : "progress", "currentStep" : 2, "totalSteps" : 8, "stepCompletion" : 0, "stepDescription" : "download_packs" },
3333
{ "type" : "info", "msg" : "Downloading packs for:" },
3434
{ "type" : "info", "msg" : " - test-bundle" },
35-
EOM
36-
)
37-
expected_output2=$(cat <<-EOM
38-
{ "type" : "progress", "currentStep" : 2, "totalSteps" : 8, "stepCompletion" : 100, "stepDescription" : "download_packs" },
3935
{ "type" : "info", "msg" : "Finishing packs extraction..." },
36+
{ "type" : "progress", "currentStep" : 2, "totalSteps" : 8, "stepCompletion" : 100, "stepDescription" : "download_packs" },
4037
{ "type" : "progress", "currentStep" : 3, "totalSteps" : 8, "stepCompletion" : -1, "stepDescription" : "extract_packs" },
4138
{ "type" : "progress", "currentStep" : 3, "totalSteps" : 8, "stepCompletion" : 100, "stepDescription" : "extract_packs" },
4239
{ "type" : "progress", "currentStep" : 4, "totalSteps" : 8, "stepCompletion" : 0, "stepDescription" : "validate_fullfiles" },
@@ -93,7 +90,6 @@ test_setup() {
9390
EOM
9491
)
9592
assert_in_output "$expected_output1"
96-
assert_in_output "$expected_output2"
9793

9894
}
9995
#WEIGHT=3

test/functional/only_in_ci_slow/update-slow-server.bats

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@ test_setup() {
3535
Downloading packs for:
3636
- test-bundle
3737
Error: Curl - File incompletely downloaded - '.*/100/pack-test-bundle-from-10.tar'
38+
Waiting 10 seconds before retrying the download
39+
Retry #1 downloading from .*/100/pack-test-bundle-from-10.tar
3840
Finishing packs extraction...
39-
Curl - Starting download retry #1 for .*/100/pack-test-bundle-from-10.tar
40-
Curl - Resuming download for '.*/100/pack-test-bundle-from-10.tar'
41-
Error: Curl - Range command not supported by server, download resume disabled - '.*/100/pack-test-bundle-from-10.tar'
42-
Curl - Starting download retry #2 for .*/100/pack-test-bundle-from-10.tar
4341
Statistics for going from version 10 to version 100:
4442
changed bundles : 1
4543
new bundles : 0

test/functional/only_in_ci_system/add-no-disk-space.bats

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,6 @@ test_setup() {
130130
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
131131
Error: Curl - Error downloading to local file - 'http://localhost:$(get_web_server_port "$TEST_NAME")/$TEST_NAME/web-dir/10/.*.tar'
132132
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
133-
Error: Curl - Error downloading to local file - 'http://localhost:$(get_web_server_port "$TEST_NAME")/$TEST_NAME/web-dir/10/.*.tar'
134-
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
135-
Error: Curl - Error downloading to local file - 'http://localhost:$(get_web_server_port "$TEST_NAME")/$TEST_NAME/web-dir/10/.*.tar'
136-
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
137-
Error: Curl - Error downloading to local file - 'http://localhost:$(get_web_server_port "$TEST_NAME")/$TEST_NAME/web-dir/10/.*.tar'
138-
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
139-
Error: Curl - Error downloading to local file - 'http://localhost:$(get_web_server_port "$TEST_NAME")/$TEST_NAME/web-dir/10/.*.tar'
140-
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
141-
Error: Curl - Error downloading to local file - 'http://localhost:$(get_web_server_port "$TEST_NAME")/$TEST_NAME/web-dir/10/.*.tar'
142-
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
143-
Error: Curl - Error downloading to local file - 'http://localhost:$(get_web_server_port "$TEST_NAME")/$TEST_NAME/web-dir/10/.*.tar'
144-
Error: Curl - Check free space for $ABS_TEST_DIR/testfs/state\\?
145133
Error: Could not download some files from bundles, aborting bundle installation
146134
Failed to install 1 of 1 bundles
147135
EOM

test/functional/os-install/install-json.bats

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,8 @@ test_setup() {
3131
{ "type" : "progress", "currentStep" : 2, "totalSteps" : 9, "stepCompletion" : 0, "stepDescription" : "download_packs" },
3232
{ "type" : "info", "msg" : "Downloading packs for:" },
3333
{ "type" : "info", "msg" : " - os-core" },
34-
EOM
35-
)
36-
expected_output2=$(cat <<-EOM
37-
{ "type" : "progress", "currentStep" : 2, "totalSteps" : 9, "stepCompletion" : 100, "stepDescription" : "download_packs" },
3834
{ "type" : "info", "msg" : "Finishing packs extraction..." },
35+
{ "type" : "progress", "currentStep" : 2, "totalSteps" : 9, "stepCompletion" : 100, "stepDescription" : "download_packs" },
3936
{ "type" : "progress", "currentStep" : 3, "totalSteps" : 9, "stepCompletion" : -1, "stepDescription" : "extract_packs" },
4037
{ "type" : "progress", "currentStep" : 3, "totalSteps" : 9, "stepCompletion" : 100, "stepDescription" : "extract_packs" },
4138
{ "type" : "progress", "currentStep" : 4, "totalSteps" : 9, "stepCompletion" : 0, "stepDescription" : "check_files_hash" },
@@ -61,13 +58,16 @@ test_setup() {
6158
{ "type" : "info", "msg" : " 0 of 2 missing files were not installed" },
6259
{ "type" : "progress", "currentStep" : 9, "totalSteps" : 9, "stepCompletion" : -1, "stepDescription" : "run_postupdate_scripts" },
6360
{ "type" : "info", "msg" : "Calling post-update helper scripts" },
64-
{ "type" : "warning", "msg" : "helper script ($ABS_TEST_DIR/testfs/target-dir/usr/bin/clr-boot-manager) not found, it will be skipped" },
61+
EOM
62+
)
63+
expected_output2=$(cat <<-EOM
6564
{ "type" : "info", "msg" : " Installation successful" },
6665
{ "type" : "progress", "currentStep" : 9, "totalSteps" : 9, "stepCompletion" : 100, "stepDescription" : "run_postupdate_scripts" },
6766
{ "type" : "end", "section" : "os-install", "status" : 0 }
6867
]
6968
EOM
7069
)
70+
7171
assert_in_output "$expected_output1"
7272
assert_in_output "$expected_output2"
7373

test/functional/os-install/install-no-fullfile-fallback.bats

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ test_setup() {
2727
Downloading missing manifests...
2828
Downloading packs for:
2929
- os-core
30-
Finishing packs extraction...
3130
Error: zero pack downloads failed
3231
Checking for corrupt files
3332
Validate downloaded files

test/functional/os-install/install-no-packs.bats

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ test_setup() {
2727
Downloading missing manifests...
2828
Downloading packs for:
2929
- os-core
30-
Finishing packs extraction...
3130
Error: zero pack downloads failed
3231
Checking for corrupt files
3332
Validate downloaded files

test/functional/os-install/install-statedir-cache-offline.bats

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,14 @@ test_setup() {
156156
Overriding version and content URLs with https://localhost
157157
Installing OS version 10
158158
Downloading missing manifests...
159-
Error: Failed to connect to update server: https://localhost
159+
Error: Failed to connect to update server: https://localhost/10/pack-os-core-from-0.tar
160160
Possible solutions for this problem are:
161161
.Check if your network connection is working
162162
.Fix the system clock
163163
.Run 'swupd info' to check if the urls are correct
164164
.Check if the server SSL certificate is trusted by your system \\('clrtrust generate' may help\\)
165+
Downloading packs for:
166+
- os-core
165167
Error: zero pack downloads failed
166168
Checking for corrupt files
167169
Validate downloaded files

test/functional/update/update-json.bats

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,8 @@ test_setup() {
3737
{ "type" : "progress", "currentStep" : 3, "totalSteps" : 10, "stepCompletion" : 0, "stepDescription" : "download_packs" },
3838
{ "type" : "info", "msg" : "Downloading packs for:" },
3939
{ "type" : "info", "msg" : " - test-bundle" },
40-
EOM
41-
)
42-
expected_output2=$(cat <<-EOM
43-
{ "type" : "progress", "currentStep" : 3, "totalSteps" : 10, "stepCompletion" : 100, "stepDescription" : "download_packs" },
4440
{ "type" : "info", "msg" : "Finishing packs extraction..." },
41+
{ "type" : "progress", "currentStep" : 3, "totalSteps" : 10, "stepCompletion" : 100, "stepDescription" : "download_packs" },
4542
{ "type" : "progress", "currentStep" : 4, "totalSteps" : 10, "stepCompletion" : -1, "stepDescription" : "extract_packs" },
4643
{ "type" : "progress", "currentStep" : 4, "totalSteps" : 10, "stepCompletion" : 100, "stepDescription" : "extract_packs" },
4744
{ "type" : "progress", "currentStep" : 5, "totalSteps" : 10, "stepCompletion" : -1, "stepDescription" : "prepare_for_update" },
@@ -76,7 +73,7 @@ test_setup() {
7673
{ "type" : "info", "msg" : "Calling post-update helper scripts" },
7774
EOM
7875
)
79-
expected_output3=$(cat <<-EOM
76+
expected_output2=$(cat <<-EOM
8077
\{ "type" : "info", "msg" : "Update took ... seconds, 0 MB transferred" \},
8178
\{ "type" : "info", "msg" : "Update successful - System updated from version 10 to version 20" \},
8279
\{ "type" : "progress", "currentStep" : 10, "totalSteps" : 10, "stepCompletion" : 100, "stepDescription" : "run_postupdate_scripts" \},
@@ -85,8 +82,7 @@ test_setup() {
8582
EOM
8683
)
8784
assert_in_output "$expected_output1"
88-
assert_in_output "$expected_output2"
89-
assert_regex_in_output "$expected_output3"
85+
assert_regex_in_output "$expected_output2"
9086

9187
}
9288
#WEIGHT=5

0 commit comments

Comments
 (0)