Drew DeVault: 1 Split download and extract into separate steps 3 files changed, 93 insertions(+), 38 deletions(-)
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.alpinelinux.org/~alpine/devel/patches/3186/mbox | git am -3Learn more about email & git
This splits apk_db_install_pkg into two steps: APK_INSTALL_DOWNLOAD and APK_INSTALL_COMMIT, and accepts a bitfield of steps to perform. apk_solver_commit_changeset performs these steps as one if the cache is disabled. Otherwise, it downloads everything, then commits everything, in two steps. By downloading everything upfront, we reduce the risk of leaving the system in a half-installed state should network issues come up partway through the upgrade. Additionally, this makes it safe to interrupt apk during the download step without a similar partially-updated-system problem. --- To onlookers: this could use some testing, please install it locally and let me know about any issues you run into. src/apk_database.h | 8 +++++ src/commit.c | 73 ++++++++++++++++++++++++++++++++-------------- src/database.c | 50 +++++++++++++++++++++---------- 3 files changed, 93 insertions(+), 38 deletions(-) diff --git a/src/apk_database.h b/src/apk_database.h index 2c8bdda..ee9f64b 100644 --- a/src/apk_database.h +++ b/src/apk_database.h @@ -12,6 +12,8 @@ #ifndef APK_PKGDB_H #define APK_PKGDB_H +#include <stdint.h> + #include "apk_version.h" #include "apk_hash.h" #include "apk_archive.h" @@ -192,6 +194,11 @@ typedef union apk_database_or_void { void *ptr; } apk_database_t __attribute__ ((__transparent_union__)); +enum apk_install_operation { + APK_INSTALL_DOWNLOAD = 1 << 0, + APK_INSTALL_COMMIT = 1 << 1, +}; + struct apk_name *apk_db_get_name(struct apk_database *db, apk_blob_t name); struct apk_name *apk_db_query_name(struct apk_database *db, apk_blob_t name); int apk_db_get_tag_id(struct apk_database *db, apk_blob_t tag); @@ -261,6 +268,7 @@ int apk_db_cache_foreach_item(struct apk_database *db, apk_cache_item_cb cb); int apk_db_install_pkg(struct apk_database *db, struct apk_package *oldpkg, struct apk_package *newpkg, + uint32_t install_ops, apk_progress_cb cb, void *cb_ctx); void apk_name_foreach_matching(struct apk_database *db, struct apk_string_array *filter, unsigned int match, diff --git a/src/commit.c b/src/commit.c index 4a94ff5..410c5c4 100644 --- a/src/commit.c +++ b/src/commit.c @@ -27,7 +27,7 @@ static inline int pkg_available(struct apk_database *db, struct apk_package *pkg } static int print_change(struct apk_database *db, struct apk_change *change, - int cur, int total) + uint32_t step, int cur, int total) { struct apk_name *name; struct apk_package *oldpkg = change->old_pkg; @@ -41,9 +41,22 @@ static int print_change(struct apk_database *db, struct apk_change *change, name = newpkg ? newpkg->name : oldpkg->name; if (oldpkg == NULL) { - msg = "Installing"; + switch (step) { + case APK_INSTALL_DOWNLOAD | APK_INSTALL_COMMIT: + msg = "Installing"; + break; + case APK_INSTALL_DOWNLOAD: + msg = "Fetching"; + break; + case APK_INSTALL_COMMIT: + msg = "Extracting"; + break; + } oneversion = newpkg->version; } else if (newpkg == NULL) { + if (!(step & APK_INSTALL_COMMIT)) { + return TRUE; + } msg = "Purging"; oneversion = oldpkg->version; } else if (newpkg == oldpkg) { @@ -55,6 +68,9 @@ static int print_change(struct apk_database *db, struct apk_change *change, } else if (change->old_repository_tag != change->new_repository_tag) { msg = "Updating pinning"; } + if (msg && !(step & APK_INSTALL_COMMIT)) { + msg = "Fetching"; + } oneversion = newpkg->version; } else { r = apk_pkg_version_compare(newpkg, oldpkg); @@ -69,6 +85,9 @@ static int print_change(struct apk_database *db, struct apk_change *change, msg = "Upgrading"; break; } + if (msg && !(step & APK_INSTALL_COMMIT)) { + msg = "Fetching"; + } } if (msg == NULL) return FALSE; @@ -259,7 +278,7 @@ int apk_solver_commit_changeset(struct apk_database *db, struct apk_changeset *changeset, struct apk_dependency_array *world) { - struct progress prog; + struct progress prog, _prog; struct apk_change *change; char buf[32], size_unit; ssize_t size_diff = 0; @@ -321,26 +340,36 @@ int apk_solver_commit_changeset(struct apk_database *db, if (run_commit_hooks(db, PRE_COMMIT_HOOK) == -2) return -1; + uint32_t steps[3] = { APK_INSTALL_DOWNLOAD, APK_INSTALL_COMMIT, 0 }; + if (!apk_db_cache_active(db)) { + steps[0] = APK_INSTALL_DOWNLOAD | APK_INSTALL_COMMIT; + steps[1] = 0; + } + + memcpy(&_prog, &prog, sizeof(prog)); /* Go through changes */ - foreach_array_item(change, changeset->changes) { - r = change->old_pkg && - (change->old_pkg->ipkg->broken_files || - change->old_pkg->ipkg->broken_script); - if (print_change(db, change, prog.done.changes, prog.total.changes)) { - prog.pkg = change->new_pkg; - progress_cb(&prog, 0); - - if (!(apk_flags & APK_SIMULATE) && - ((change->old_pkg != change->new_pkg) || - (change->reinstall && pkg_available(db, change->new_pkg)))) { - r = apk_db_install_pkg(db, change->old_pkg, change->new_pkg, - progress_cb, &prog) != 0; + for (int i = 0; steps[i]; ++i) { + memcpy(&prog, &_prog, sizeof(prog)); + foreach_array_item(change, changeset->changes) { + r = change->old_pkg && change->old_pkg->ipkg && + (change->old_pkg->ipkg->broken_files || + change->old_pkg->ipkg->broken_script); + if (print_change(db, change, steps[i], prog.done.changes, prog.total.changes)) { + prog.pkg = change->new_pkg; + progress_cb(&prog, 0); + + if (!(apk_flags & APK_SIMULATE) && + ((change->old_pkg != change->new_pkg) || + (change->reinstall && pkg_available(db, change->new_pkg)))) { + r = apk_db_install_pkg(db, change->old_pkg, change->new_pkg, + steps[i], progress_cb, &prog) != 0; + } + if (r == 0 && change->new_pkg && change->new_pkg->ipkg) + change->new_pkg->ipkg->repository_tag = change->new_repository_tag; } - if (r == 0 && change->new_pkg && change->new_pkg->ipkg) - change->new_pkg->ipkg->repository_tag = change->new_repository_tag; + errors += r; + count_change(change, &prog.done); } - errors += r; - count_change(change, &prog.done); } apk_print_progress(prog.total.bytes + prog.total.packages, prog.total.bytes + prog.total.packages); @@ -625,7 +654,7 @@ void apk_solver_print_errors(struct apk_database *db, * b-1: * satisfies: world[b] * conflicts: a-1[foo] - * + * * c-1: * satisfies: world[a] * conflicts: c-1[foo] (self-conflict by providing foo twice) @@ -641,7 +670,7 @@ void apk_solver_print_errors(struct apk_database *db, * satisfies lists all dependencies that is not satisfiable by * any other selected version. or all of them with -v. */ - + apk_error("unsatisfiable constraints:"); /* Construct information about names */ diff --git a/src/database.c b/src/database.c index c699444..6741b67 100644 --- a/src/database.c +++ b/src/database.c @@ -2767,7 +2767,8 @@ static void apk_db_migrate_files(struct apk_database *db, static int apk_db_unpack_pkg(struct apk_database *db, struct apk_installed_package *ipkg, - int upgrade, apk_progress_cb cb, void *cb_ctx, + int upgrade, uint32_t install_opts, + apk_progress_cb cb, void *cb_ctx, char **script_args) { struct install_ctx ctx; @@ -2777,7 +2778,7 @@ static int apk_db_unpack_pkg(struct apk_database *db, struct apk_package *pkg = ipkg->pkg; char file[PATH_MAX]; char tmpcacheitem[128], *cacheitem = &tmpcacheitem[tmpprefix.len]; - int r, filefd = AT_FDCWD, need_copy = FALSE; + int r = 0, filefd = AT_FDCWD, need_copy = FALSE; if (pkg->filename == NULL) { repo = apk_db_select_repo(db, pkg); @@ -2797,20 +2798,31 @@ static int apk_db_unpack_pkg(struct apk_database *db, } need_copy = TRUE; } - if (!apk_db_cache_active(db)) + if (!apk_db_cache_active(db)) { need_copy = FALSE; - - bs = apk_bstream_from_fd_url(filefd, file); - if (IS_ERR_OR_NULL(bs)) { - r = PTR_ERR(bs); - if (r == -ENOENT && pkg->filename == NULL) - r = -EAPKSTALEINDEX; - goto err_msg; - } - if (need_copy) { + } else { + /* Attempt to install from cache, if present */ apk_blob_t b = APK_BLOB_BUF(tmpcacheitem); apk_blob_push_blob(&b, tmpprefix); apk_pkg_format_cache_pkg(b, pkg); + cache_bs = apk_bstream_from_file(db->cache_fd, cacheitem); + if (!IS_ERR_OR_NULL(cache_bs)) { + bs = cache_bs; + need_copy = FALSE; + } + } + + if (!bs) { + bs = apk_bstream_from_fd_url(filefd, file); + if (IS_ERR_OR_NULL(bs)) { + r = PTR_ERR(bs); + if (r == -ENOENT && pkg->filename == NULL) + r = -EAPKSTALEINDEX; + goto err_msg; + } + } + + if (need_copy && (install_opts & APK_INSTALL_DOWNLOAD)) { cache_bs = apk_bstream_tee(bs, db->cache_fd, tmpcacheitem, 1, NULL, NULL); if (!IS_ERR_OR_NULL(cache_bs)) bs = cache_bs; @@ -2831,11 +2843,14 @@ static int apk_db_unpack_pkg(struct apk_database *db, }; apk_sign_ctx_init(&ctx.sctx, APK_SIGN_VERIFY_IDENTITY, &pkg->csum, db->keys_fd); tar = apk_bstream_gunzip_mpart(bs, apk_sign_ctx_mpart_cb, &ctx.sctx); - r = apk_tar_parse(tar, apk_db_install_archive_entry, &ctx, TRUE, &db->id_cache); + if (install_opts & APK_INSTALL_COMMIT) + r = apk_tar_parse(tar, apk_db_install_archive_entry, &ctx, TRUE, &db->id_cache); + else + r = apk_tar_parse(tar, apk_sign_ctx_verify_tar, &ctx.sctx, FALSE, &db->id_cache); apk_sign_ctx_free(&ctx.sctx); apk_istream_close(tar); - if (need_copy) { + if (need_copy && (install_opts & APK_INSTALL_DOWNLOAD)) { if (r == 0) { renameat(db->cache_fd, tmpcacheitem, db->cache_fd, cacheitem); pkg->repos |= BIT(APK_REPOSITORY_CACHED); @@ -2854,7 +2869,8 @@ err_msg: } int apk_db_install_pkg(struct apk_database *db, struct apk_package *oldpkg, - struct apk_package *newpkg, apk_progress_cb cb, void *cb_ctx) + struct apk_package *newpkg, uint32_t install_opts, + apk_progress_cb cb, void *cb_ctx) { char *script_args[] = { NULL, NULL, NULL, NULL }; struct apk_installed_package *ipkg; @@ -2870,6 +2886,8 @@ int apk_db_install_pkg(struct apk_database *db, struct apk_package *oldpkg, /* Just purging? */ if (oldpkg != NULL && newpkg == NULL) { + if (!(install_opts & APK_INSTALL_COMMIT)) + goto ret_r; ipkg = oldpkg->ipkg; if (ipkg == NULL) goto ret_r; @@ -2893,7 +2911,7 @@ int apk_db_install_pkg(struct apk_database *db, struct apk_package *oldpkg, } if (newpkg->installed_size != 0) { - r = apk_db_unpack_pkg(db, ipkg, (oldpkg != NULL), + r = apk_db_unpack_pkg(db, ipkg, (oldpkg != NULL), install_opts, cb, cb_ctx, script_args); if (r != 0) { if (oldpkg != newpkg) -- 2.24.1
Timo Teras <timo.teras@iki.fi>Drew, Thanks for working on this. This is something I was planning to implement myself in Jan/Feb. Though, I'm first doing file format changes and hoping to publish that work still this year. I hope you saw the heads up I sent some days ago about that. Along with that, I am planing to fix things like this. Though, I was planning to change the overall system so that each package is extracted immediately. But the final moving of files to in place with the real name would done only after *all* packaged were extracted. I am not decided yet if downloading packages to cache should be the default when cache is enabled. Perhaps yes, especially since we are seeing more setups where the same cache is shared across containers. However, I'd rather not overload apk_db_unpack_pkg() for this purpose. I was hoping that the commit code would instead schedule pure "fetch" jobs. (This is by the way a place I'm hoping to rewrite completely to be more "job" based and use thread pools to do the download/fetch/extract/verify pipelines in separate threads. Then synchronize and do the final renameat() as last step.) Perhaps I should write my "road map" email sooner than later to indicate how I'd hope things to be done. Timo On Mon, 23 Dec 2019 09:12:20 -0500 Drew DeVault <sir@cmpwn.com> wrote: