Repository navigation
fix: subscribe and unsubscribe no longer need sticky sessions - #65
Merged
Merged
Conversation
With a transport, only broadcasts went through the bus. A subscribe request that reached an instance not holding the client's stream returned false, so the client got a 400, and an unsubscribe there left the holder subscribed. Every multi-replica deployment needed sticky sessions. The receiving instance now authorizes the subscription with its own context. When it holds the stream, it subscribes it as before; otherwise it publishes the subscription on the bus and returns true. The holder applies it without authorizing again, and the other instances ignore it. Unsubscribe always publishes on the bus and still applies locally. The subscribe event fires once, on the receiving instance. Without a transport, an unknown uid still returns false. With a transport, subscribe now returns true for a uid that no instance holds, because the receiving instance cannot know. Subscription messages use their own bus channel, "<transport channel>::subscriptions". Older versions broadcast every message on the transport channel to the channel's clients, which would leak another client's uid during a rolling deploy. Until the holder is upgraded, a relayed subscription to it is lost. An unsubscribe then a subscribe on the same channel that reach two instances can still be applied out of order.
Subscribe and unsubscribe published their relay without waiting, so an instance could answer before the bus had the message. When a client unsubscribed through one instance and then subscribed through another, the second message could reach the bus first, and the holder ended up unsubscribed. They now wait for the publish. When a client waits for each response before its next request, as @adonisjs/transmit-client does, the bus receives its changes in order. This costs one round trip to the bus per subscribe or unsubscribe that goes through it. A change that reaches the holder itself is still applied at once, and can be applied before an earlier change from another instance that the bus has not delivered yet.
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.
With a transport (Redis, MQTT), a
POST /__transmit/subscribethat reaches an instance that does not hold the client's stream returns 400. An unsubscribe on that instance returns 204 but the holder stays subscribed, and keeps sending messages that the client ignores. Only broadcasts go through the bus, so every multi-replica deployment needs sticky sessions.Now subscriptions go through the bus too:
subscribeevent fires once, on the instance that receives the request, with its context.Subscription messages use their own bus channel,
<transport channel>::subscriptions. Older versions broadcast every message ontransmit::broadcastto the channel's clients. If subscriptions used that channel, an old instance would send another client'suidto its clients during a rolling deploy.Behavior changes
subscribereturns true (HTTP 204) for auidthat no instance holds, because the receiving instance cannot know. Without a transport, it still returns false.Known limit
Subscribe and unsubscribe answer only after the bus has the relay. When a client waits for each response before its next request, as
@adonisjs/transmit-clientdoes, two changes that go through two other instances reach the holder in order.Not covered: a change that reaches the holder itself is applied at once. If an earlier change from another instance is still on its way through the bus, the holder applies the two out of order. For example, the client unsubscribes through A, then subscribes through B (the holder): B can apply the subscribe, then the unsubscribe from A. The client then receives nothing on that channel until its stream reconnects. There is no guard for this.
Tests
Four tests in
tests/transmit.spec.ts:subscribeevent fires only on the receiving instance with its context, and that nouidgoes ontransmit::broadcast. It fails without the change, when only the unsubscribe relay is removed, when the holder authorizes again, when subscriptions usetransmit::broadcast, and when the holder also emits the event.uidis refused.I also ran the packed package end to end on Valkey 8.1 with the Redis transport. The stream was on one instance and the POSTs went to the other. I used
@adonisjs/transmit3.0.2 in an AdonisJS 7 app, driven from Chromium with@adonisjs/transmit-client1.1.0. The base fails these cases and this branch passes them, 5 runs of 5 on each side. The browser runs used the first commit. After the second commit, I ran the Node scenario on Valkey again (two instances, rolling deploy, Valkey restart), and all 9 cases pass.Not covered: Redis Cluster, MQTT, Firefox and Safari.