-
Notifications
You must be signed in to change notification settings - Fork 82
Certified Nodes usage telemetry #6017
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.
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
Conversation
Gethers certified nodes count every 24hrs Dual implementation, telemetry and a standalone housekeeping task need to pick one before merging
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6017 +/- ##
==========================================
- Coverage 76.79% 76.64% -0.16%
==========================================
Files 384 387 +3
Lines 19439 19557 +118
Branches 4671 4703 +32
==========================================
+ Hits 14928 14989 +61
- Misses 4511 4568 +57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The consumer for this is all up and running now Only outstanding question is should the scopes and upload URL be configurable? |
|
@knolleary if you get a second |
| lastSeenAt: { [Op.gte]: new Date(now - 1000 * 60 * 60 * 24) } | ||
| } | ||
| }) | ||
| for (const dev of runningDevices) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to review this in more detail - not sure how this'll handle devices in dev mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't easily count devices in dev mode until they push a snapshot, which this should pick up. But if they add nodes and stay in devMode we will never know
This should do 20 per second to reduce database load
|
@knolleary this now runs the queries in batches of 20 every second until complete to reduce load on db, we can tune the time period if 20/s is still too fast |
| const instancePromise = new Promise((resolve, reject) => { | ||
| // uses getRuntimeSettings to include template merge | ||
| let instanceOffset = 0 | ||
| const instanceInterval = setInterval(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If, for any reason, it takes longer than 1s to process the whole batch of twenty, this will end up scheduling batches in a parallel - putting even more load on the DB than if it was just doing them all in a single batch.
Lets revert the batching change here and go back to a simple iteration over the list. Add some log statements at start/end of the task so we can check back on how long it takes to run on FFC. We can then decide if it needs batching etc.
knolleary
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for bouncing this again. See me other comment - lets revert the batching change (for both instances and devices), add some logging and see how it does on FFC.
part of https://github.com/FlowFuse/customer/issues/392
Description
Gethers certified nodes count every 24hrs
Dual implementation, telemetry anda standalone housekeeping taskneed to pick one before mergingRelated Issue(s)
https://github.com/FlowFuse/customer/issues/392
Checklist
flowforge.yml?FlowFuse/helmto update ConfigMap TemplateFlowFuse/CloudProjectto update values for Staging/ProductionLabels
area:migrationlabel