docs: a transport must preserve publish order, and why - #48
Merged
Conversation
MqttDeviceTransport says nothing about ordering, because neither transport the SDK ships with can violate it: paho's own thread and asyncio_driver each pump one client, so publishes reach the wire in the order they were made. A transport written against the protocol directly need not, and one that starts a task per publish() hands ordering to the scheduler. That matters because ordering is a guarantee the SDK maintains on a producer's behalf. A device's $description precedes the $state=ready that vouches for it, and refresh_tree() publishes a device's own $state after the children it announces -- which is what 0.18.1 fixed. Those are quality-of-implementation, not protocol: a consumer must never depend on them, and doc/consuming-a-homie-tree.md says so at length, since publish order does not survive retention. That is precisely why the warning cannot live there. The consumer guide addresses the other party, so it will never reach a transport author, who can silently drop a guarantee the SDK spends effort meeting. Also records the teardown consequence. A publish() that enqueues and returns is entirely legitimate -- every return in the protocol is typed `object` because the SDK discards them -- but Device.stop() publishes the final $state and returns without flushing, so a queueing transport needs a drain point before its client closes, or that message is lost behind it. Raised by @cayossarian on #46, from building a natively-async transport where the hazard is real rather than theoretical: it holds by luck under a fast broker and breaks under a slow first publish. Docs only; no source changed.
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.
Documents an obligation the injected-transport contract implies but never states. Docs only; no source changed.
Raised by @cayossarian on #46, from building a natively-async
MqttDeviceTransportwhere the hazard is real rather than theoretical.The gap
MqttDeviceTransportsays nothing about ordering, and until now it did not need to: neither transport shipped with the SDK can violate it. paho's own thread andasyncio_drivereach pump one client, so publishes reach the wire in the order they were made.A transport written against the protocol directly need not. One that starts a task per
publish()hands ordering to the scheduler.Why that is a real loss and not a nitpick
Ordering is a guarantee the SDK maintains on a producer's behalf:
$descriptionprecedes the$state=readythat vouches for itrefresh_tree()publishes a device's own$stateafter the children it announces, which is what 0.18.1 fixed (refresh_tree() publishes a device's $state before its children, so the root announces ready to an empty tree #31 / fix: publish a device's $state after its children in refresh_tree #32)Those are quality-of-implementation rather than protocol. A consumer must never depend on them, and
doc/consuming-a-homie-tree.mdargues that at length, since publish order does not survive retention.Which is exactly why the warning cannot live in that document. It addresses consumers, so it will never reach a transport author — who can therefore drop a guarantee the SDK spends effort meeting, silently, while every document that discusses ordering is telling the other party not to rely on it.
Also recorded: the teardown consequence
A
publish()that enqueues and returns is entirely legitimate. Every return in the protocol is typedobjectprecisely because the SDK discards them (transport.py:29). ButDevice.stop()publishes the final$stateand returns without flushing, so a queueing transport needs a drain point of its own before its client closes, or that last message is lost behind it.Measured by @cayossarian on his implementation: remove the drain and the root's final
$statenever reaches the broker, with a log line as the only trace.Note on scope
This documents the rule. A worked example of a natively-async transport would be more useful still, and @cayossarian has the only known implementation; that is asked for separately on #46 and is his to decline. This paragraph is not blocked on it.
Related: #46 proposes the teardown API whose absence makes the reach-around necessary in the first place.