Differential D32570
freebsd-update: efficiently handle no updates in fetch Authored by tux2bsd_protonmail.com on Oct 20 2021, 7:26 AM. Tags None Referenced Files
Subscribers
Details
Markedly improve "fetch" when there are no updates. On IO bound hardware, in my case a Raspberry Pi 3, this improvement has reduced runtime duration of freebsd-update fetch from 1m20s down to 1.5s. Even on fast SSD hardware freebsd-update fetch currently takes ~15 seconds, this changes reduces the time to sub second. (git commit message has more) Tested with run combinations: (edit: some historic output available: https://github.com/freebsd/freebsd-src/pull/543)
Diff Detail
Event TimelineComment Actions This will break very badly if freebsd-update fetch gets interrupted. The file tag is created at the end of fetch_metadata so the fact that it exists does not imply that the fetch completed. We need to be very very careful to avoid the situation where an out-of-date system emits No updates are available to fetch erroneously. Comment Actions It is NOT being repurposed, a force fetch will run as it did prior. The entire point of checking for that was to allow it to run as before. (edit: at the time I didn't think I was repurposing -F) If tag and tag.new are different in any way or either tag or tag.new do not exist fetch will run as normal. An out of date system cannot emit No updates are available to fetch This patch was built up locally by me then further on GitHub with some great input there over the last few weeks. It hasn't simply landed here untested / unvetted.
Comment Actions btw, once we get this right, if you push the git changes to github, and I'll land it from there. Comment Actions @imp a new squashed commit pushed to github (should be the same as now) https://github.com/freebsd/freebsd-src/pull/543/commits/30e1ba1213684df5ec22eee4b20aee9e7efbf48b Comment Actions Ah, I see it now. So in this patch (currently), "No updates are available to fetch": How can it be detected that fetch_run ran to completion AND allows this patch to bypass unnecessary runs... something like rm -f fetch.run.completed (at the beginning of fetch_metadata) (edit: that was what I thought out loud, I went with a similar approach) I'm glad to see @cperciva is a fan of shaving time off things. The reason I was sharing this patch to begin with was the drastic reduction of run times. Comment Actions introduces a way to test that fetch_run ran to completion in a way that allows the new tag match test Comment Actions Some more explanation in the comments. Removed the function, it was one line so it seemed a bit silly. Comment Actions Some more explanation in the comments. Removed the function, it was one line so it seemed a bit silly. (typo, sorry) Comment Actions Put in what I hope is a better commented explanation. Sorry if there is too much activity on this, I'm just trying to get it right. Let me know if I'm too noisy.
Comment Actions Take out the bit about "Force", simply focus on tag/tag.new and fetch_run prior completion. Comment Actions I thought more about what you said here and have taken out the '-F' check, my patch doesn't need to concern itself with an unfinished upgrade. -F Force freebsd-update fetch to proceed in the case of an
unfinished upgrade.The goal of my patch has been to make fetch efficient (in the case of no upstream updates). Comment Actions Thanks, I think this solves the 'fetch dies partway through' issue. I'm not entirely sure that it handles the case of 'a previous fetch downloaded updates which have not yet been installed' though -- it looks like it now exits with "No updates are available to fetch", which is a change from the previous behaviour. In particular I'd like to make sure that
since those are both very common usage patterns. Comment Actions Regarding #2, This use case was something I tested with along the way (this VM was deliberately out of date for this). It looks like this: root@pull543:~ # freebsd-update fetch install src component not installed, skipped Looking up update.FreeBSD.org mirrors... 2 mirrors found. Fetching metadata signature for 13.0-RELEASE from update2.freebsd.org... done. No updates are available to fetch. Creating snapshot of existing boot environment... done. Installing updates... Regarding #1, "a previous fetch downloaded updates which have not yet been installed" My thought around #1 would be if "No updates are available to fetch." is emited in "cron" then call "updatesready" for the email unless it has "No updates are available to install." If that sounds good for the "cron" mode I can work toward that, please let me know? root@pull543:~ # freebsd-update updatesready src component not installed, skipped No updates are available to install. (pull543 was the github pull request number) Comment Actions Hmm. Maybe it would be clearer if "No updates are available to fetch" continued with " but previously fetched updates are available to install"? I can imagine users being very confused as to why their system is installing updates after saying that none are available.
I think it's useful for the email sent by freebsd-update cron to list the updates -- this may help users to decide how urgent it is to install said updates, and simply sending an email saying "No updates are available to fetch. There are updates available to install." loses that information. I'm leaning towards saying that the best solution here is to split fetch_create_manifest into fetch_create_manifest and fetch_print_summary and then call fetch_print_summary if we determine that nothing new needs to be fetched but previously fetched updates are waiting to be installed. Comment Actions Colin suggested splitting the manifest function in two but I'm not seeing how to do that, I'm sure it's possible but at this stage that has me stumped. What I have done instead is capture the manifest output(s) into a file to present at the right times (immediately when called, and when the tags match). The output is removed in a few places to avoid cross contamination, fetch vs upgrade for example. This output of fetch is passed through to cron , the cron subroutine is reverted prior to my attempts with D32570. Comment Actions Much cleaner. Only thing I'm not sure about is how we handle upgrade.
Comment Actions Will this work if the -b basedir flag is used? Jails can be updated this way, and we don't want to fail to fetch updates for a jail simply because we've already fetched updates for the host. Comment Actions Sorry, another thought: What happens in the following sequence of events?
Comment Actions
I've made two simple changes that will force an "old fashioned fetch" after either install or rollback. As for the '-b' option, I'll have to think about that as I'm not a jail user.
Comment Actions Please elaborate on "This will still break"? That lacks a reasonable explanation. I don't have an environment with jails and will have to spend some time setting that up at some point when I get some more time to look at this. Comment Actions https://wiki.freebsd.org/2021FoundationCFI search for "freebsd-update improvements", I did ask for help - hopefully someone can take up where I left off. Comment Actions Just to note, I haven't forgotten about this; I've just been very busy the past two weeks due to re:Invent. I promise I'll be coming back to this before 13.1-RELEASE. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||