Changed default to CleanSession(false) to preserve previous subscriptions

[kamermans]

See issue #17 : upon reconnection, all existing subscriptions are lost. The two ways I've found to fix this are to set CleanSession(false) in the MQTT client, or to explicitly resubscribe to all channels in the OnConnect() handler. I don't see a good reason for the session to be cleaned during a reconnection, so I've added this PR to change the default of MQTT CleanSession from true to false. I've also added an option to allow the user to change it back with WithCleanSession().

Related discussion and docs:
eclipse-paho/paho.mqtt.golang#22
https://godoc.org/github.com/eclipse/paho.mqtt.golang#ClientOptions.SetCleanSession

[Florimond]

Does it subscribe again using the exact same channel string, including options, like "last"?
I suppose it does, which can be good or bad depending on the use case...

The problem I have with this flag, is that it seems to imply things on the broker side, and the Emitter broker does not implement anything regarding this on its side.

[kamermans]

Good question! The paho documentation says that it "does not tell the server to clear the session when you reconnect", so the idea is that the MQTT server already knows your session state (based on your UUID in the case of emitter-go), and is willing to resume it. Whether this means last is preserved, I don't know, some testing may be required to determine that.

[Florimond]

Yes, that's what I wrote in the edited version of my answer, just before you wrote yours.
Unless I missed something, Emitter doesn't implement anything regarding this flag on its side. To me, the way to go is to handle both subscription and re-subscription in the connection handler.

[kamermans]

That is true, and I am on the fence here as well. Personally, I've moved my Subscribe() functions into my OnConnect() handlers to be sure I'm subscribed every time I (re)connect.

The main problem I have with the current implementation is that it's basically impossible to guarantee that you are ever subscribed. A microsecond after you subscribe, there could be a network interruption that triggers an auto-reconnection, then you're connected but not subscribed anymore.

Currently I'm using emitter across about 40 servers in three continents, and they never fail to find a way to disconnect, even though I've spread emitter cluster members around the earth to reduce latency.

[Florimond]

I don't see it... If you set the OnConnect handler before sending your first connection request, and only subscribe in this handler: how then could you "miss" the subscription?

[kamermans]

Sorry, I was referring to my code before I used OnConnect. Now it is working properly.

[MichaelHipp]

Shouldn't some of this be in the documentation, so every person who comes along doesn't have to re-discover it for themselves?

[kamermans]

Definitely, although if this is just a documentation issue, we'll need to ensure that basically every code example uses this paradigm.

[kamermans]

Also, it's probably still nice to be able to set CleanSession(), so if you don't want to turn it on by default, I can just change the default to true in this PR and people can set it to false if they so choose.

[MichaelHipp]

IMHO, it should be set to false by default. That seems like the "least surprise" option to me. But in any event it needs to be documented.

[Florimond]

I haven't tested the clean session set to false in Go, but I just did a quick test with the Python version of Paho MQTT, and it doesn't re-subscribes. Which is in consistent with what I read here about the Python lib .

the subscription information is remembered by the broker, and so the client doesn’t need to subscribe again

And about MQTT in general, on the IBM knowledge center :

If you set MqttConnectOptions.cleanSession to false before connecting, any subscriptions the client creates are added to all the subscriptions that existed for the client before it connected. All the subscriptions remain active when the client disconnects.

To me, it's clear everything is supposed to happen on the broker side. And the fact that the Go library takes upon itself to re-subscribe is a exceptional behavior. It does not follow the rule.

I think that the best we could do is to add to every emitter lib some "resubscribe" parameter, and implement the behavior independent of the MQTT's clean session flag.

[kamermans]

Thanks for the response!

the fact that the Go library takes upon itself to re-subscribe is a exceptional behavior. It does not follow the rule.

Agreed that we probably don't want to resubscribe explicitly in the emitter wrapper. The argument for CleanSession=true would be that the emitter client already makes some non-default choices, like always passing a randomly generated user ID.

I'm understanding you correctly, you agree that the default behavior is that the user must explicitly re-subscribe to their channel(s) in Connect() or set CleanSession() to false (via this PR) to ensure a persistent subscription; and that the default behavior should be as it is today to avoid the Go client from diverging from the de-facto standard (and the other official Emitter clients). That seems reasonable to me, despite the weirdness. One argument for your position is that you really don't want a ton of persistent connections to be tracked by the broker from temporary clients.

It seems like this PR is still valuable, but I should change the default back to true, and add some documentation to make it clear that long-lived / persistent clients should set CleanSession(false) or resubscribe in OnConnect().

[Florimond]

It seems like this PR is still valuable, but I should change the default back to true, and add some documentation to make it clear that long-lived / persistent clients should set CleanSession(false) or resubscribe in OnConnect().

Ok. Let's do this. I'll merge the pull request then...

[kamermans]

Ok, sounds good. I'll get it changed and let you know.

[kamermans]

@Florimond @MichaelHipp I've updated the PR to maintain the current options and left the ability for users to set cleanSession=false if they so choose. I've also added some prominent notes and examples to the README that should draw attention to the fact that sessions are not automatically resubscribed after a reconnection.

What do you think?

[Florimond]

While all information given about cleansession is true for MQTT brokers, it should be clarified that Emitter does not implement these behaviors, and that that's the underlying go MQTT lib that takes upon itself to resubscribe. Messages however won't be sent unless requested upon resubscription (with last or what I'm going to add now "WithFrom" and "WithUntil").

[kamermans]

My understanding is that all MQTT clients resubscribe like this since it's a core feature of MQTT. I did not think it was go-specific.