refactor(mqtt): drop the redundant sleep between OBSERVE registrations - #59
Merged
Merged
Conversation
_run_session_inner slept 50ms after every subscribe() call. subscribe() opens with self.pace(), which withholds the send until _min_req_interval has passed since the last datagram, and the default rate limit is 5 req/s. The sleep was always shorter than the wait that followed it, so no registration ever left the host at a different time because of it. Noted on #51 and #53.
mbillow
added a commit
to mbillow/SmartThings-Local
that referenced
this pull request
Aug 23, 2026
refresh_observes() dropped every OBSERVE registration in a tight loop. Unlike the teardown dereg in close(), which wants out quickly and leaves a session nobody will use again, this one runs against a session that has to keep working afterwards — and an unpaced OBSERVE burst is what wedges an appliance until something forces a new session (LocalThings#396). The two sleeps that stood in for pacing are gone with it. subscribe() paces its own send since QuiteYellow#51, so the 50ms between registrations was always shorter than the wait that followed it — the same redundancy QuiteYellow#59 removed from the bridge's registration loop, at the sibling call site. The 100ms between the two sweeps is subsumed the same way, by the pace inside the first subscribe(). Note this does not fix the connect-time OBSERVE burst in #396 on its own: that path is the bridge's registration loop, which QuiteYellow#51 already paces.
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
1 participant
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.
_run_session_innerslept 50ms after everysubscribe()call.subscribe()opens withself.pace(), which withholds the send until_min_req_intervalhas passed since the last datagram, and the default rate limit is 5 req/s. The sleep was always shorter than the wait that followed it, so no registration ever left the host at a different time because of it.Noted on #51 and #53.
Deployed to the reference bridge (one dryer, one oven) and soaked. Discarding the window that contains the connect, five 60-second windows on each: dryer 95-96 ok, oven 220-229 ok, no errors, no ping failures and no timeouts on either. Those counts sit inside the pre-change baseline of 94-96 and 222-229. The dryer seeded without the DTLS listener stall a container recreate sometimes causes.
Latency in the steady-state windows also matches baseline (dryer p_max 256-272ms against 221-283ms, oven 2011-2346ms against 1838-1980ms). The two windows containing a link sweep ran above their baseline counterparts, dryer 1151ms against 913ms and oven 3447ms against 2635ms, with
slowcounts unchanged. Removing a delay cannot add latency, so I read those two as device-side sweep variance. The numbers are here either way.mqtt_demo/is outside the distributed package, so this does not reach the wheel.