-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
VTAdmin: Show throttled status in workflows #16308
VTAdmin: Show throttled status in workflows #16308
Conversation
Signed-off-by: Rohit Nayak <[email protected]>
Signed-off-by: Rohit Nayak <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Rohit Nayak <[email protected]>
…omponents/routes/workflow/Workflow.module.scss src/components/routes/workflow/Workflow.tsx src/components/routes/workflow/WorkflowStreams.tsx src/components/routes/Workflows.tsx Signed-off-by: Rohit Nayak <[email protected]>
Signed-off-by: Rohit Nayak <[email protected]>
…ime columns don't get cleared once it is no longer throttled Signed-off-by: Rohit Nayak <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16308 +/- ##
==========================================
+ Coverage 68.71% 68.72% +0.01%
==========================================
Files 1544 1547 +3
Lines 198011 198270 +259
==========================================
+ Hits 136064 136269 +205
- Misses 61947 62001 +54 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Rohit Nayak <[email protected]>
Current test failures are due to the issue with installing xtrabackup ... |
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.
I think there's still a misunderstanding baked in here about what the throttler info means. If a workflow was throttled 6 hours ago, that would be reflected in the record. That does NOT mean, however, that we've been throttled for 6 hours. In fact it means the opposite, we haven't been throttled in 6 hours.
// If the stream has been throttled for more than 5 seconds, show it as throttled. | ||
Number(stream?.throttler_status?.time_throttled?.seconds) > | ||
Date.now() / 1000 - ThrottleThresholdSeconds |
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.
Aren't we showing it if it's been throttled in the last minute? I don't see how we can answer "for more than 5 seconds" or how we're attempting to do that here.
if (numThrottled === 1) { | ||
throttledApp = | ||
stream?.throttler_status?.component_throttled?.toString(); | ||
throttledFrom = stream?.throttler_status?.time_throttled; |
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.
throttledAt
would be more accurate here
} | ||
if (numThrottled > 0) { | ||
streamDescription = ''; | ||
streamState = 'Throttled'; |
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.
Recently Throttled
would be correct.
' throttled in ' + | ||
throttledApp + | ||
' ' + | ||
formatRelativeTime(throttledFrom?.seconds) + |
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.
Another place where we're treating the last throttled time as if we've been throttled since then when it only indicates when we were last throttled. You can tell if you're constantly being throttled by watching this value over time.
Signed-off-by: Rohit Nayak <[email protected]>
Signed-off-by: Rohit Nayak <[email protected]>
Signed-off-by: Rohit Nayak <[email protected]>
Signed-off-by: Rohit Nayak <[email protected]>
Signed-off-by: Rohit Nayak <[email protected]>
e58999b
to
5cd70c8
Compare
Signed-off-by: Rohit Nayak <[email protected]>
Signed-off-by: Rohit Nayak <[email protected]>
…ifiercc config Signed-off-by: Rohit Nayak <[email protected]>
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.
thanks Rohit!
Description
When a VReplication workflow is throttled, the app that caused the throttling and the time of the last throttle are recorded in the
_vt.vreplication
table. Currently those attributes are only shown in the json output tab of the workflow screen and are not easy to find.The throttler app and time are not reset once throttling has ended. So we only show a stream as throttled if it is within (currently) a minute. The throttler is updating this row on each check (every 250ms - 1s).
This PR changes the status color for a throttled running workflow to black and adds the throttler details (app name and time since throttled) to the summary workflow tooltip and to the streams details section.
It also adds a new internal link in the header to scroll down to the Streams section for convenience and visibility.
Workflow summary page
Workflow Streams Section
Related Issue(s)
#11690
Checklist
Deployment Notes