Skip to content

RSDK-10929: Log slow package download progress #5061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 11 commits into from
Jun 18, 2025
Merged

RSDK-10929: Log slow package download progress #5061

merged 11 commits into from
Jun 18, 2025
+59 −1

Conversation

aldenh-viam
Copy link
Contributor

@aldenh-viam aldenh-viam commented Jun 17, 2025 •
edited
Loading

Prints download path and download progress for package downloads. Hardcoded for now to log every 5 seconds.

Quick download shows completion message only (<5s)

2025-06-17T21:02:27.502Z INFO	rdk.package_manager packages/cloud_package_manager.go:369	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 33893493 bytes (100%) in 3.528057355s

Slow download logs progress every 5s:

2025-06-18T12:55:05.460Z INFO	rdk.package_manager packages/cloud_package_manager.go:361	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 4228252 / 33893493 bytes (12%) [477 KB/s]
2025-06-18T12:55:10.929Z INFO	rdk.package_manager packages/cloud_package_manager.go:361	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 8422556 / 33893493 bytes (25%) [582 KB/s]
2025-06-18T12:55:20.217Z INFO	rdk.package_manager packages/cloud_package_manager.go:361	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 13108380 / 33893493 bytes (39%) [547 KB/s]
2025-06-18T12:55:27.118Z INFO	rdk.package_manager packages/cloud_package_manager.go:361	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 17450140 / 33893493 bytes (51%) [562 KB/s]
2025-06-18T12:55:33.323Z INFO	rdk.package_manager packages/cloud_package_manager.go:361	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 21464220 / 33893493 bytes (63%) [574 KB/s]
2025-06-18T12:55:39.967Z INFO	rdk.package_manager packages/cloud_package_manager.go:361	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 25003164 / 33893493 bytes (74%) [566 KB/s]
2025-06-18T12:55:51.348Z INFO	rdk.package_manager packages/cloud_package_manager.go:361	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 31671452 / 33893493 bytes (93%) [567 KB/s]
2025-06-18T12:55:53.610Z INFO	rdk.package_manager packages/cloud_package_manager.go:370	/home/viam/.viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_4_7-linux-amd64.download: downloaded 33893493 bytes (100%) in 56.801073702s

@aldenh-viam aldenh-viam requested a review from a team June 17, 2025 21:22
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jun 17, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 17, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 17, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 18, 2025
dgottlieb
dgottlieb approved these changes Jun 18, 2025
View reviewed changes
Copy link
Member

@dgottlieb dgottlieb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an edge case here where if the download stalls, we stop emitting progress. But what we have here as an easy fix is perfectly fine. Compared to spinning off a background goroutine to manage the logging bit.

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 18, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 18, 2025
dgottlieb
dgottlieb reviewed Jun 18, 2025
View reviewed changes
dgottlieb
dgottlieb approved these changes Jun 18, 2025
View reviewed changes
Copy link
Member

@dgottlieb dgottlieb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still lgtm

@aldenh-viam
Copy link
Contributor Author

There's an edge case here where if the download stalls, we stop emitting progress. But what we have here as an easy fix is perfectly fine. Compared to spinning off a background goroutine to manage the logging bit.

Good point about download stalling. If >0% has already been downloaded, it can probably be inferred from prior log messages (e.g. no new updates after 30% downloaded). But a background goroutine would be the way to go if we want to fire off alarms in this case.

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 18, 2025
@viambot viambot removed the safe to test This pull request is marked safe to test from a trusted zone label Jun 18, 2025
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jun 18, 2025
cheukt
cheukt approved these changes Jun 18, 2025
View reviewed changes
Copy link
Member

@cheukt cheukt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@aldenh-viam aldenh-viam merged commit b8c7c35 into viamrobotics:main Jun 18, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgottlieb dgottlieb dgottlieb approved these changes

@cheukt cheukt cheukt approved these changes

Assignees
No one assigned
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aldenh-viam @dgottlieb @cheukt @viambot