Frontend: limit calls to /status to 0.5hz#3827
Open
Williangalvani wants to merge 1 commit intobluerobotics:masterfrom
Open
Frontend: limit calls to /status to 0.5hz#3827Williangalvani wants to merge 1 commit intobluerobotics:masterfrom
Williangalvani wants to merge 1 commit intobluerobotics:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a minimum interval between backend status checks by tracking the last time the backend was seen online and short‑circuiting new /status polling requests if they occur within that window, while preserving the existing shared status-request logic. Sequence diagram for throttled backend status checks in axios_backend_instancesequenceDiagram
actor Browser
participant AxiosBackendInstance
participant FrontendStore
participant BackendAPI
Browser->>AxiosBackendInstance: sendRequest(config)
AxiosBackendInstance->>FrontendStore: get last_backend_online_time
AxiosBackendInstance->>AxiosBackendInstance: compute online_recently
alt online_recently is true
AxiosBackendInstance-->>Browser: proceed with original request (no /status)
else online_recently is false
AxiosBackendInstance->>FrontendStore: check backend_status_request
alt backend_status_request is null
AxiosBackendInstance->>BackendAPI: GET /status
AxiosBackendInstance->>FrontendStore: setBackendStatusRequest(promise)
else backend_status_request exists
AxiosBackendInstance->>FrontendStore: reuse backend_status_request
end
FrontendStore-->>AxiosBackendInstance: backend_status_result
alt backend online
AxiosBackendInstance->>FrontendStore: setBackendOffline(false)
else backend offline or timeout
AxiosBackendInstance->>FrontendStore: setBackendOffline(true)
end
AxiosBackendInstance-->>Browser: proceed or fail based on backend status
end
Updated class diagram for FrontendStore backend status trackingclassDiagram
class FrontendStore {
+Promise~AxiosResponse~ backend_status_request
+Date last_backend_online_time
+boolean backend_offline
+string frontend_id
+string backend_status_url
+void setBackendOffline(offline boolean)
+void setBackendStatusRequest(request Promise~AxiosResponse~)
}
class AxiosBackendInstance {
+minimum_backend_status_check_interval number
+interceptRequest(config any) Promise~any~
}
FrontendStore <.. AxiosBackendInstance: uses
AxiosBackendInstance --> FrontendStore : reads last_backend_online_time
AxiosBackendInstance --> FrontendStore : updates backend_status_request
AxiosBackendInstance --> FrontendStore : calls setBackendOffline
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider storing
last_backend_online_timeas a numeric timestamp (e.g.numberfromDate.now()) rather than aDateinstance to simplify comparisons and avoid repeatednew Date().getTime()calls in the interceptor. - The
minimum_backend_status_check_intervalconstant would be clearer with units in the name (e.g...._ms) and possibly a brief comment indicating it enforces a 0.5 Hz status check rate to make the intent obvious to future readers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider storing `last_backend_online_time` as a numeric timestamp (e.g. `number` from `Date.now()`) rather than a `Date` instance to simplify comparisons and avoid repeated `new Date().getTime()` calls in the interceptor.
- The `minimum_backend_status_check_interval` constant would be clearer with units in the name (e.g. `..._ms`) and possibly a brief comment indicating it enforces a 0.5 Hz status check rate to make the intent obvious to future readers.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return false; | ||
| } | ||
|
|
||
| const minimum_backend_status_check_interval = 2000 |
Collaborator
There was a problem hiding this comment.
Suggested change
| const minimum_backend_status_check_interval = 2000 | |
| const minimum_backend_status_check_interval = 2000 // ms |
Or in the variable name (like Sourcery suggested), if that's preferable 🤷♂️
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was seeing requests to /status at ~3hz.
I don't expect huge improvements of performance or bandwidth, but this will at the very least making look at the network tab of developer tools more... sane.
Summary by Sourcery
Limit frontend backend status checks to avoid excessive /status requests.
Enhancements: