Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of SetMessageChannelDepth #339

Closed
brocaar opened this issue Jul 18, 2019 · 4 comments
Closed

Use of SetMessageChannelDepth #339

brocaar opened this issue Jul 18, 2019 · 4 comments

Comments

@brocaar
Copy link
Contributor

brocaar commented Jul 18, 2019

SetMessageChannelDepth sets the size of the internal queue that holds messages while the client is temporairily offline, allowing the application to publish when the client is reconnecting. This setting is only valid if AutoReconnect is set to true, it is otherwise ignored.


I'm not sure if I understand this feature correctly after doing some testing:

  • start up a client with SetMessageChannelDepth(2)
  • stop the MQTT broker
  • publish 5 messages (QOS=1)
  • start the MQTT broker

Instead of 2 messages, I receive 5.

  • start up a client with SetMessageChannelDepth(2)
  • stop the MQTT broker
  • publish 5 messages (QOS=0)
  • start the MQTT broker

Instead of 2 messages, I receive none.


Looking at the code:

The SetMessageChannelDepth actually sets the outbound Go channel size:

https://github.com/eclipse/paho.mqtt.golang/blob/master/client.go#L213

When the client is in re-connect state (thus temporarily disconnected), with QOS = 0 this MessageChannelDepth does not have any effect as Publish will immediately return:

https://github.com/eclipse/paho.mqtt.golang/blob/master/client.go#L575

In any other case (QOS != 0 and client is reconnecting), the message will be persisted:

https://github.com/eclipse/paho.mqtt.golang/blob/master/client.go#L597

As far as I could find, the message persistence (using the Store interface) does not share the MessageChannelDepth value so it will store an infinite number of messages.

It looks like the library will only publish to the outbound Go channel when the client is connected:

https://github.com/eclipse/paho.mqtt.golang/blob/master/client.go#L607

If I'm not mistaken, this action would block until there is a slot available in the channel (with size MessageChannelDepth) or when time.After(publishWaitTimeout) occurs.

If the above is correct, how does this SetMessageChannelDepth method relate to SetMessageChannelDepth sets the size of the internal queue that holds messages while the client is temporairily offline?

@brocaar
Copy link
Contributor Author

brocaar commented Aug 12, 2019

Any feedback on this would be appreciated 🙂

@MattBrittan
Copy link
Contributor

At the time this was introduced (back in 2016) I dont think the store was fully implemented (some code existed but it was pull request #194, in 2018, that added code to "Actually read stored message when connecting or reconnecting and send them").

So my understanding is that this option is of very limited use in the current version (and the comments need updating). At the time it was added it did what the comments say; see https://github.com/eclipse/paho.mqtt.golang/blob/617c801af238c3af2d9e72c5d4a0f02edad03ce5/client.go

@alsm
Copy link
Contributor

alsm commented Aug 29, 2019

As @MattBrittan says these are functionally useless now that the store implementations are complete. The client throws away QoS0 messages in this situation because the spec allows it and because the persistence stores use msgID as a key (and QoS0 messages don't have one). I will leave the functions in for now so as to not break anyone who happened to set them, but the values are no longer used for anything.

@alsm alsm closed this as completed Aug 29, 2019
@brocaar
Copy link
Contributor Author

brocaar commented Aug 29, 2019

says these are functionally useless now that the store implementations are complete

Maybe add a note that this function should no longer be used and will be removed in the future? 🙂 That at least avoids that somebody else will create an issue with the same question.

alsm pushed a commit to alsm/paho.mqtt.golang that referenced this issue Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants