feat(updater): download resume and progress logging

joshtrichards commented 3 months ago

Closes #570
Addresses comment on https://github.com/nextcloud/updater/issues/188#issuecomment-562037111 re: supporting RANGE to resume downloads
Establishes foundation for #188 (i.e. add real-time visibility in the UI rather than just in the log)

What it does:

Adds real-time download progress in updater.log (in 2% increments until reaching 10% then 10% increments after that)
Adds download statistics in updater.log upon completion (bytes downloaded, total time, average speed)
Detects a previously existing download and resumes it using HTTP RANGE

TODO (before merging):

[x] Re-add clean-up at the top of the extractDownload(), but only specifically of the /downloads/nextcloud/ folder

Example log (with a partial previous download detected):

[...]
2024-06-23T14:52:38+0000 bsACf4fy2k [info] downloadUpdate()
2024-06-23T14:52:38+0000 bsACf4fy2k [info] getUpdateServerResponse()
2024-06-23T14:52:38+0000 bsACf4fy2k [info] updaterServer: http://localhost:8870/
2024-06-23T14:52:38+0000 bsACf4fy2k [info] releaseChannel: daily
2024-06-23T14:52:38+0000 bsACf4fy2k [info] internal version: 30.0.0
2024-06-23T14:52:38+0000 bsACf4fy2k [info] updateURL: http://localhost:8870/?version=30x0x0xxxdailyxx2024-05-22T23%3A35%3A04%2B00%3A00+02dc1bcf36ffac7e2ff14b7a1dabcff6754f365bx8x2x18
2024-06-23T14:52:38+0000 bsACf4fy2k [info] getUpdateServerResponse response: Array
(
    [version] => 100.0.0.0
    [versionstring] => Nextcloud master
    [url] => https://download.nextcloud.com/server/daily/latest-master.zip
    [web] => https://docs.nextcloud.org/server/10/admin_manual/maintenance/manual_upgrade.html
    [autoupdater] => 1
)

2024-06-23T14:52:48+0000 bsACf4fy2k [info] storage location already exists
2024-06-23T14:52:48+0000 bsACf4fy2k [info] previous download found; resuming from 158.94MB
2024-06-23T14:52:51+0000 bsACf4fy2k [info] download progress: 2% (1015.46KB of 65.79MB)
2024-06-23T14:52:52+0000 bsACf4fy2k [info] download progress: 4% (2.31MB of 65.79MB)
2024-06-23T14:52:53+0000 bsACf4fy2k [info] download progress: 6% (3.63MB of 65.79MB)
2024-06-23T14:52:53+0000 bsACf4fy2k [info] download progress: 8% (4.98MB of 65.79MB)
2024-06-23T14:52:54+0000 bsACf4fy2k [info] download progress: 10% (6.28MB of 65.79MB)
2024-06-23T14:52:55+0000 bsACf4fy2k [info] download progress: 20% (12.83MB of 65.79MB)
2024-06-23T14:52:56+0000 bsACf4fy2k [info] download progress: 30% (19.41MB of 65.79MB)
2024-06-23T14:52:56+0000 bsACf4fy2k [info] download progress: 40% (26.04MB of 65.79MB)
2024-06-23T14:52:57+0000 bsACf4fy2k [info] download progress: 50% (32.57MB of 65.79MB)
2024-06-23T14:52:57+0000 bsACf4fy2k [info] download progress: 60% (39.15MB of 65.79MB)
2024-06-23T14:52:58+0000 bsACf4fy2k [info] download progress: 70% (45.8MB of 65.79MB)
2024-06-23T14:52:58+0000 bsACf4fy2k [info] download progress: 80% (52.32MB of 65.79MB)
2024-06-23T14:52:59+0000 bsACf4fy2k [info] download progress: 90% (58.88MB of 65.79MB)
2024-06-23T14:52:59+0000 bsACf4fy2k [info] download progress: 100% (65.46MB of 65.79MB)
2024-06-23T14:53:00+0000 bsACf4fy2k [info] download stats: size=65.79MB bytes; total_time=11.61 secs; avg speed=5.67MB/sec
2024-06-23T14:53:00+0000 bsACf4fy2k [info] end of downloadUpdate()

Most material change this makes internally: We no longer delete the /downloads/ storage location if it exists. Instead we delete only the extracted archive contents sub-folder (/downloads/nextcloud/).

This was obviously necessary for the resume to work (since we want the Archive zip).

We still clean-up /downloads/nextcloud/ (just not the whole /downloads/ folder) in case the Updater fails at later steps and leaves the extracted contents around.

Follow-up PR ideas:

Show the download progress in the CLI mode of the Updater
Show the download progress in the Web mode of the Updater
A background job that cleans up old zip Archives periodically (stale Archive zip files are a possibility even without this PR)
To get the broadest benefit of always being able to re-use a previous download (whether full or partial) after unsuccessful Updater runs, we should eliminate the clean-up of the zip Archive within the extractor step. Maybe move the zip Archive clean-up to the finalize() step. The downside being that ~250MB would be consumed until the Updater finishes rather than being freed up at the extraction stage. I don't know how to decide whether that's reasonable or not for all the different environments we try to support. So I tried to keep this PR conservative. Technically that space already had be available in the environment because the extraction step itself requires space for the Archive + the extracted contents since both must co-exist until the extraction is successful. Therefore I think it'd be safe move that clean-up to finalize(). But with the Updater being a very sensitive area I figured... maybe a follow-up PR tweaking that makes more sense. :)

joshtrichards commented 3 months ago

Does that mean that previous downloads for other upgrades will stack up and eat up disk space?

Under some circumstances, yes. But we're also already leaving Archive files around today whenever there are failures before/ during the extraction step (either until the next Updater run or forever if a manual update is used after the Updater fails). If those are of concern we should probably just add a background job to clean them up.

After successful Updater runs, the /downloads/ folder (including /downloads/nextcloud/) is already left empty and what I removed was a no-op under those circumstances
After unsuccessful Updater runs, this PR takes advantage of the prior download (full or partial) being in /downloads/ by resuming or re-using it rather than deleting it only to re-download it again immediately after
After unsuccessful Updater runs, it's possible the extracted content folder (/downloads/nextcloud/) is not empty and that is bad [This is the issue I hinted at in my follow-up edit. It's probably the main thing that fully wiping of /downloads/ here was saving us from.]

To keep this PR from introducing any new potential problems we were previously saving ourselves from, I'll re-add clean-up at the top of the extractDownload(), but only specifically of the /downloads/nextcloud/ folder. This will still leave any Archive zip files in /downloads/ alone.

github-actions[bot] commented 3 months ago

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

come-nc commented 2 months ago

Looks good, but seems like there is a lot of duplicated code. Could we move them to third file?

It’s not duplicated index.php is built with make and contains the other file. Unless I misunderstood what you’re talking about?

nextcloud / updater

feat(updater): download resume and progress logging #571