From 4e6fb28d0a3fe83a6de7095042e9f2092dbf8053 Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Thu, 25 Feb 2016 16:09:12 -0800 Subject: [PATCH] Improve parallel pack generation Because pack generation is a time-consuming task, running this task in parallel is preferable. However, if we are generating two or more packs with the same "from" version at the same time, the multiple jobs will use the same pack staging directory, leading to corruption. My solution for now is to use a staging dir with a name derived from both "from" and "to" versions so that there are no collisions. A long-term fix will be to implement some kind of locking mechanism to prevent *identical* packs from being generated simultaneously, but for now, using more unique staging directories will improve the situation. Signed-off-by: Patrick McCarty --- src/pack.c | 60 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/pack.c b/src/pack.c index d337ce6..dd847b2 100644 --- a/src/pack.c +++ b/src/pack.c @@ -39,28 +39,31 @@ #include "swupd.h" -static void empty_pack_stage(int full, int version, char *module) +static void empty_pack_stage(int full, int from_version, int to_version, char *module) { char *cmd; char *path; int ret; // clean any stale data (eg: re-run after a failure) - string_or_die(&cmd, "rm -rf %s/%s/%i/", packstage_dir, module, version); + string_or_die(&cmd, "rm -rf %s/%s/%i_to_%i/", packstage_dir, module, + from_version, to_version); ret = system(cmd); if (ret) { - fprintf(stderr, "Failed to clean %s/%s/%i\n", - packstage_dir, module, version); + fprintf(stderr, "Failed to clean %s/%s/%i_to_%i\n", + packstage_dir, module, from_version, to_version); exit(EXIT_FAILURE); } free(cmd); if (!full) { // (re)create module/version/{delta,staged} - string_or_die(&path, "%s/%s/%i/delta", packstage_dir, module, version); + string_or_die(&path, "%s/%s/%i_to_%i/delta", packstage_dir, module, + from_version, to_version); g_mkdir_with_parents(path, S_IRWXU | S_IRWXG); free(path); - string_or_die(&path, "%s/%s/%i/staged", packstage_dir, module, version); + string_or_die(&path, "%s/%s/%i_to_%i/staged", packstage_dir, module, + from_version, to_version); g_mkdir_with_parents(path, S_IRWXU | S_IRWXG); free(path); } @@ -69,14 +72,15 @@ static void empty_pack_stage(int full, int version, char *module) /* * for very small files (eh.. lets make that all), we untar them for pack purposes */ -static void explode_pack_stage(int version, char *module) +static void explode_pack_stage(int from_version, int to_version, char *module) { DIR *dir; struct dirent *entry; struct stat buf; char *path; - string_or_die(&path, "%s/%s/%i/staged", packstage_dir, module, version); + string_or_die(&path, "%s/%s/%i_to_%i/staged", packstage_dir, module, + from_version, to_version); g_mkdir_with_parents(path, S_IRWXU | S_IRWXG); dir = opendir(path); if (!dir) { @@ -99,8 +103,8 @@ static void explode_pack_stage(int version, char *module) continue; } - string_or_die(&path, "%s/%s/%i/staged/%s", - packstage_dir, module, version, entry->d_name); + string_or_die(&path, "%s/%s/%i_to_%i/staged/%s", packstage_dir, module, + from_version, to_version, entry->d_name); ret = stat(path, &buf); if (ret) { free(path); @@ -113,9 +117,9 @@ static void explode_pack_stage(int version, char *module) * the resulting pack is slightly smaller, and in addition, we're saving CPU * time on the client... */ - string_or_die(&tar, TAR_COMMAND " --directory=%s/%s/%i/staged " TAR_WARN_ARGS " " + string_or_die(&tar, TAR_COMMAND " --directory=%s/%s/%i_to_%i/staged " TAR_WARN_ARGS " " TAR_PERM_ATTR_ARGS " -xf %s", - packstage_dir, module, version, path); + packstage_dir, module, from_version, to_version, path); ret = system(tar); if (!ret) { unlink(path); @@ -140,7 +144,7 @@ static void prepare_pack(struct packdata *pack) pack->end_manifest = manifest_from_file(pack->to, pack->module); - empty_pack_stage(0, pack->from, pack->module); + empty_pack_stage(0, pack->from, pack->to, pack->module); match_manifests(manifest, pack->end_manifest); @@ -165,7 +169,8 @@ static void make_pack_full_files(struct packdata *pack) char *from, *to; /* hardlink each file that is in but not in */ string_or_die(&from, "%s/%i/files/%s.tar", staging_dir, file->last_change, file->hash); - string_or_die(&to, "%s/%s/%i/staged/%s.tar", packstage_dir, pack->module, pack->from, file->hash); + string_or_die(&to, "%s/%s/%i_to_%i/staged/%s.tar", packstage_dir, + pack->module, pack->from, pack->to, file->hash); ret = link(from, to); if (ret) { if (errno != EEXIST) { @@ -331,12 +336,13 @@ static int make_final_pack(struct packdata *pack) /* locate delta, check if the diff it's from is >= */ string_or_die(&from, "%s/%i/delta/%i-%i-%s", staging_dir, file->last_change, file->peer->last_change, file->last_change, file->hash); - string_or_die(&to, "%s/%s/%i/delta/%i-%i-%s", packstage_dir, pack->module, - pack->from, file->peer->last_change, file->last_change, file->hash); + string_or_die(&to, "%s/%s/%i_to_%i/delta/%i-%i-%s", packstage_dir, + pack->module, pack->from, pack->to, file->peer->last_change, + file->last_change, file->hash); string_or_die(&tarfrom, "%s/%i/files/%s.tar", staging_dir, file->last_change, file->hash); - string_or_die(&tarto, "%s/%s/%i/staged/%s.tar", packstage_dir, pack->module, - pack->from, file->hash); + string_or_die(&tarto, "%s/%s/%i_to_%i/staged/%s.tar", packstage_dir, + pack->module, pack->from, pack->to, file->hash); ret = stat(from, &stat_delta); if (ret) { @@ -387,7 +393,7 @@ static int make_final_pack(struct packdata *pack) if (pack->fullcount > 0) { /* untar all files for smaller size and less cpu use on client */ - explode_pack_stage(pack->from, pack->module); + explode_pack_stage(pack->from, pack->to, pack->module); } /* now... link in the Manifest pack */ @@ -404,8 +410,9 @@ static int make_final_pack(struct packdata *pack) create_manifest_delta(pack->from, pack->to, pack->module); } - string_or_die(&to, "%s/%s/%i/Manifest-%s-delta-from-%i", - packstage_dir, pack->module, pack->from, pack->module, pack->from); + + string_or_die(&to, "%s/%s/%i_to_%i/Manifest-%s-delta-from-%i", packstage_dir, + pack->module, pack->from, pack->to, pack->module, pack->from); ret = link(from, to); if (ret) { @@ -429,8 +436,8 @@ static int make_final_pack(struct packdata *pack) create_manifest_delta(pack->from, pack->to, "MoM"); } - string_or_die(&to, "%s/%s/%i/Manifest-%s-delta-from-%i", - packstage_dir, pack->module, pack->from, "MoM", pack->from); + string_or_die(&to, "%s/%s/%i_to_%i/Manifest-%s-delta-from-%i", packstage_dir, + pack->module, pack->from, pack->to, "MoM", pack->from); ret = link(from, to); if (ret) { @@ -443,9 +450,10 @@ static int make_final_pack(struct packdata *pack) /* tar the staging directory up */ LOG(NULL, "starting tar for pack", "%s: %i to %i", pack->module, pack->from, pack->to); - string_or_die(&tar, TAR_COMMAND " " TAR_PERM_ATTR_ARGS " --directory=%s/%s/%i/ " + string_or_die(&tar, TAR_COMMAND " " TAR_PERM_ATTR_ARGS " --directory=%s/%s/%i_to_%i/ " "--numeric-owner -Jcf %s/%i/pack-%s-from-%i.tar delta staged", - packstage_dir, pack->module, pack->from, staging_dir, pack->to, pack->module, pack->from); + packstage_dir, pack->module, pack->from, pack->to, staging_dir, pack->to, + pack->module, pack->from); ret = system(tar); free(tar); LOG(NULL, "finished tar for pack", "%s: %i to %i", pack->module, pack->from, pack->to); @@ -464,7 +472,7 @@ static int make_final_pack(struct packdata *pack) /* and clean up */ free_manifest(pack->end_manifest); - empty_pack_stage(1, pack->from, pack->module); + empty_pack_stage(1, pack->from, pack->to, pack->module); LOG(NULL, "pack complete", "%s: %i to %i", pack->module, pack->from, pack->to); return ret;