Skip to content
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

RSDK-2886 Reduce config ticker and simplify completeConfig goroutine #2291

Merged
merged 2 commits into from
Apr 27, 2023
Merged

RSDK-2886 Reduce config ticker and simplify completeConfig goroutine #2291

merged 2 commits into from
Apr 27, 2023

Conversation

benjirewis
Copy link
Member

RSDK-2886

[idea] We should be able to reduce the config timer (renamed configTicker) to tick every 5 seconds. This should decrease test running time and increase the frequency with which we're able to detect changes in cloud configs/remotes. I think we can also use a single triggerConfig channel (bool seemed overly expressive given we only ever send true).

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Apr 27, 2023
@benjirewis benjirewis requested review from edaniels and cheukt April 27, 2023 16:06
@benjirewis benjirewis marked this pull request as ready for review April 27, 2023 17:46
@benjirewis
Copy link
Member Author

Timing locally, this change seems to lower testing time by about 30 seconds. That's not significant, so I can leave RSDK-2886 open for now.

@benjirewis benjirewis requested a review from cheukt April 27, 2023 18:15
@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 Apr 27, 2023
@benjirewis
Copy link
Member Author

benjirewis commented Apr 27, 2023 •
edited
Loading

@edaniels , @cheukt brought up a good point offline about 5 seconds being a pretty drastic change from 25 seconds. He was worried about thrashing/overuse of memory if we quintuple the rate at which completeConfig runs. Any thoughts on a slightly higher tick like 10 or 15 seconds?

@edaniels
Copy link
Contributor

edaniels commented Apr 27, 2023 •
edited
Loading

5 seconds in computer time is a while. what memory would be getting overused or thrash?

@cheukt
Copy link
Member

cheukt commented Apr 27, 2023

I'm not concerned about that so much as trying too often and unnecessarily - if a component is down it probably takes longer than 5s to fully restart and reinitialize.

@github-actions
Copy link
Contributor

Code Coverage

Package Line Rate Delta Health
go.viam.com/rdk/components/arm 64% 0.00% ➖
go.viam.com/rdk/components/arm/fake 29% 0.00% ❌
go.viam.com/rdk/components/arm/universalrobots 41% 0.00% ❌
go.viam.com/rdk/components/arm/wrapper 21% 0.00% ❌
go.viam.com/rdk/components/arm/xarm 23% 0.00% ❌
go.viam.com/rdk/components/arm/yahboom 6% 0.00% ❌
go.viam.com/rdk/components/audioinput 44% 0.00% ❌
go.viam.com/rdk/components/base 41% 0.00% ❌
go.viam.com/rdk/components/base/agilex 63% 0.00% ➖
go.viam.com/rdk/components/base/boat 40% 0.00% ❌
go.viam.com/rdk/components/base/fake 52% 0.00% ➖
go.viam.com/rdk/components/base/wheeled 75% 0.00% ✅
go.viam.com/rdk/components/board 60% 0.00% ➖
go.viam.com/rdk/components/board/fake 40% 0.00% ❌
go.viam.com/rdk/components/board/genericlinux 20% 0.00% ❌
go.viam.com/rdk/components/board/numato 19% 0.00% ❌
go.viam.com/rdk/components/board/pi 50% 0.00% ➖
go.viam.com/rdk/components/camera 57% 0.00% ➖
go.viam.com/rdk/components/camera/align 58% 0.00% ➖
go.viam.com/rdk/components/camera/fake 74% 0.00% ✅
go.viam.com/rdk/components/camera/ffmpeg 82% 0.00% ✅
go.viam.com/rdk/components/camera/rtsp 46% +0.88% ❌
go.viam.com/rdk/components/camera/transformpipeline 77% 0.00% ✅
go.viam.com/rdk/components/camera/videosource 34% 0.00% ❌
go.viam.com/rdk/components/encoder 57% 0.00% ➖
go.viam.com/rdk/components/encoder/ams 64% 0.00% ➖
go.viam.com/rdk/components/encoder/fake 83% 0.00% ✅
go.viam.com/rdk/components/encoder/incremental 80% 0.00% ✅
go.viam.com/rdk/components/encoder/single 86% 0.00% ✅
go.viam.com/rdk/components/gantry 56% 0.00% ➖
go.viam.com/rdk/components/gantry/multiaxis 87% 0.00% ✅
go.viam.com/rdk/components/gantry/oneaxis 87% 0.00% ✅
go.viam.com/rdk/components/generic 79% 0.00% ✅
go.viam.com/rdk/components/gripper 69% 0.00% ➖
go.viam.com/rdk/components/input 88% 0.00% ✅
go.viam.com/rdk/components/input/fake 93% 0.00% ✅
go.viam.com/rdk/components/input/gpio 85% 0.00% ✅
go.viam.com/rdk/components/motor 71% 0.00% ✅
go.viam.com/rdk/components/motor/dimensionengineering 65% 0.00% ➖
go.viam.com/rdk/components/motor/dmc4000 70% 0.00% ✅
go.viam.com/rdk/components/motor/fake 53% 0.00% ➖
go.viam.com/rdk/components/motor/gpio 59% +0.61% ➖
go.viam.com/rdk/components/motor/gpiostepper 52% 0.00% ➖
go.viam.com/rdk/components/motor/tmcstepper 64% 0.00% ➖
go.viam.com/rdk/components/motor/ulnstepper 53% 0.00% ➖
go.viam.com/rdk/components/movementsensor 76% 0.00% ✅
go.viam.com/rdk/components/movementsensor/adxl345 66% 0.00% ➖
go.viam.com/rdk/components/movementsensor/cameramono 41% 0.00% ❌
go.viam.com/rdk/components/movementsensor/gpsnmea 51% 0.00% ➖
go.viam.com/rdk/components/movementsensor/gpsrtk 28% 0.00% ❌
go.viam.com/rdk/components/movementsensor/mpu6050 84% 0.00% ✅
go.viam.com/rdk/components/posetracker 71% 0.00% ✅
go.viam.com/rdk/components/sensor 52% 0.00% ➖
go.viam.com/rdk/components/sensor/ultrasonic 43% 0.00% ❌
go.viam.com/rdk/components/servo 62% 0.00% ➖
go.viam.com/rdk/components/servo/gpio 72% 0.00% ✅
go.viam.com/rdk/config 79% 0.00% ✅
go.viam.com/rdk/control 57% 0.00% ➖
go.viam.com/rdk/data 77% 0.00% ✅
go.viam.com/rdk/examples/customresources/demos/remoteserver 0% 0.00% ❌
go.viam.com/rdk/grpc 25% 0.00% ❌
go.viam.com/rdk/internal/cloud 100% 0.00% ✅
go.viam.com/rdk/ml 67% 0.00% ➖
go.viam.com/rdk/ml/inference 71% 0.00% ✅
go.viam.com/rdk/module 76% 0.00% ✅
go.viam.com/rdk/module/modmanager 80% 0.00% ✅
go.viam.com/rdk/motionplan 71% +0.05% ✅
go.viam.com/rdk/operation 82% 0.00% ✅
go.viam.com/rdk/pointcloud 69% +0.07% ➖
go.viam.com/rdk/protoutils 49% 0.00% ❌
go.viam.com/rdk/referenceframe 72% 0.00% ✅
go.viam.com/rdk/resource 75% 0.00% ✅
go.viam.com/rdk/rimage 77% 0.00% ✅
go.viam.com/rdk/rimage/depthadapter 94% 0.00% ✅
go.viam.com/rdk/rimage/transform 75% 0.00% ✅
go.viam.com/rdk/rimage/transform/cmd/extrinsic_calibration 67% 0.00% ➖
go.viam.com/rdk/robot 86% 0.00% ✅
go.viam.com/rdk/robot/client 82% 0.00% ✅
go.viam.com/rdk/robot/framesystem 67% 0.00% ➖
go.viam.com/rdk/robot/impl 82% +0.06% ✅
go.viam.com/rdk/robot/packages 80% 0.00% ✅
go.viam.com/rdk/robot/server 53% 0.00% ➖
go.viam.com/rdk/robot/web 64% 0.00% ➖
go.viam.com/rdk/robot/web/stream 87% 0.00% ✅
go.viam.com/rdk/services/baseremotecontrol 50% 0.00% ➖
go.viam.com/rdk/services/baseremotecontrol/builtin 82% 0.00% ✅
go.viam.com/rdk/services/datamanager 65% 0.00% ➖
go.viam.com/rdk/services/datamanager/builtin 85% 0.00% ✅
go.viam.com/rdk/services/datamanager/datacapture 73% 0.00% ✅
go.viam.com/rdk/services/datamanager/datasync 0% 0.00% ❌
go.viam.com/rdk/services/mlmodel 83% 0.00% ✅
go.viam.com/rdk/services/mlmodel/tflitecpu 83% 0.00% ✅
go.viam.com/rdk/services/motion 51% 0.00% ➖
go.viam.com/rdk/services/motion/builtin 85% 0.00% ✅
go.viam.com/rdk/services/navigation 53% 0.00% ➖
go.viam.com/rdk/services/sensors 81% 0.00% ✅
go.viam.com/rdk/services/sensors/builtin 95% 0.00% ✅
go.viam.com/rdk/services/shell 11% 0.00% ❌
go.viam.com/rdk/services/slam 93% 0.00% ✅
go.viam.com/rdk/services/slam/fake 90% 0.00% ✅
go.viam.com/rdk/services/vision 35% 0.00% ❌
go.viam.com/rdk/services/vision/colordetector 56% 0.00% ➖
go.viam.com/rdk/services/vision/detectionstosegments 67% 0.00% ➖
go.viam.com/rdk/services/vision/mlvision 70% 0.00% ➖
go.viam.com/rdk/services/vision/radiusclustering 59% 0.00% ➖
go.viam.com/rdk/session 97% 0.00% ✅
go.viam.com/rdk/spatialmath 84% 0.00% ✅
go.viam.com/rdk/utils 71% 0.00% ✅
go.viam.com/rdk/vision 26% 0.00% ❌
go.viam.com/rdk/vision/chess 80% 0.00% ✅
go.viam.com/rdk/vision/delaunay 87% 0.00% ✅
go.viam.com/rdk/vision/keypoints 92% 0.00% ✅
go.viam.com/rdk/vision/objectdetection 82% 0.00% ✅
go.viam.com/rdk/vision/odometry 60% 0.00% ➖
go.viam.com/rdk/vision/odometry/cmd 0% 0.00% ❌
go.viam.com/rdk/vision/segmentation 49% 0.00% ❌
go.viam.com/rdk/web/server 26% 0.00% ❌
Summary 65% (21459 / 32922) +0.01% ➖

@benjirewis
Copy link
Member Author

We now lower testing time by about 1m45s.

@edaniels
Copy link
Contributor

I'm not concerned about that so much as trying too often and unnecessarily - if a component is down it probably takes longer than 5s to fully restart and reinitialize.

Oh I think that's fine

@benjirewis benjirewis requested a review from cheukt April 27, 2023 19:56
cheukt
cheukt approved these changes Apr 27, 2023
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

@benjirewis benjirewis merged commit dbf6abf into viamrobotics:main Apr 27, 2023
@benjirewis benjirewis deleted the RSDK-2886 branch April 27, 2023 22:35
bazile-clyde pushed a commit to bazile-clyde/rdk that referenced this pull request Jan 5, 2024
@benjirewis
RSDK-2886 Reduce config ticker and simplify completeConfig goroutine (v…
0fcb04d 
…iamrobotics#2291)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edaniels edaniels edaniels 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
@benjirewis @edaniels @cheukt @viambot