luci-app-pbr-1.2.3: refresh stale service status after Save & Apply - #33
Merged
Conversation
Saving settings fires pbr's procd config.change trigger, which reloads the service asynchronously. LuCI reloads the page as soon as the apply completes, so getInitStatus() lands mid-reload and reports the service as stopped -- and nothing ever re-checked it, leaving a stale "Stopped" in the status box until the user refreshed the page by hand. The service control buttons already handled this via pollServiceStatus() in pbr/status.js; the plain settings-save path went through LuCI's generic form flow and had no equivalent. Re-check the status from render(), but only while it looks like that transient state (enabled but not running), stopping as soon as it settles and giving up after ~90s. Intervals ramp 1.5s -> 3s -> 5s, so the common case flips within a couple of seconds without competing for CPU with the reload being waited on. Gating on enabled && !running matters for cost: getInitStatus() is expensive on the router, since the rpcd handler builds a fresh pbr instance per request and so re-runs full platform detection plus an `nft list table` dump on every call. A normally-running service therefore costs no extra RPC calls at all. Two deliberate choices worth recording: - setTimeout rather than LuCI's poll module, matching pollServiceStatus() in pbr/status.js. A registered poll drives LuCI's global auto-refresh indicator, which then sits at "Paused" once unregistered, reading as though the page had stalled. - The status box is re-rendered only once the state settles, not on every tick, so the service control buttons inside it aren't torn out from under the user while they are looking at a stopped service. Known limitation: this covers stale "Stopped" only. If the page loads while the service is running and it later fails to come back, the box stays stale until a manual refresh -- the trade for zero polling in the healthy case. A service the user stopped deliberately also re-checks for ~90s before giving up, which is bounded and self-terminating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Erik Conijn <egc112@msn.com>
Collaborator
Author
|
Co-Pilot comment: |
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.
Saving settings fires pbr's procd config.change trigger, which reloads the service asynchronously. LuCI reloads the page as soon as the apply completes, so getInitStatus() lands mid-reload and reports the service as stopped -- and nothing ever re-checked it, leaving a stale "Stopped" in the status box until the user refreshed the page by hand. The service control buttons already handled this via pollServiceStatus() in pbr/status.js; the plain settings-save path went through LuCI's generic form flow and had no equivalent.
Re-check the status from render(), but only while it looks like that transient state (enabled but not running), stopping as soon as it settles and giving up after ~90s. Intervals ramp 1.5s -> 3s -> 5s, so the common case flips within a couple of seconds without competing for CPU with the reload being waited on.
Gating on enabled && !running matters for cost: getInitStatus() is expensive on the router, since the rpcd handler builds a fresh pbr instance per request and so re-runs full platform detection plus an
nft list tabledump on every call. A normally-running service therefore costs no extra RPC calls at all.Two deliberate choices worth recording:
Known limitation: this covers stale "Stopped" only. If the page loads while the service is running and it later fails to come back, the box stays stale until a manual refresh -- the trade for zero polling in the healthy case. A service the user stopped deliberately also re-checks for ~90s before giving up, which is bounded and self-terminating.