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

SetWriteTimeout doesn't work right? #436

Open
Tockra opened this issue Jun 30, 2020 · 7 comments
Open

SetWriteTimeout doesn't work right? #436

Tockra opened this issue Jun 30, 2020 · 7 comments

Comments

@Tockra
Copy link

Tockra commented Jun 30, 2020

Hi Guys,

I created a mqtt.Client with SetWriteTimeout(time.Second * 10).

But when I call

//...
token := (*mqttConnection).Publish(b.topic, b.qos , false, msg)
		if token.Wait(); token.Error() != nil {
			log.Println("MQTT SEND ERROR: ", token.Error())
			return token.Error()
		}
		log.Println("Sended")

Then I blocked mqtt broker with iptables. After that Publish blocks and doesn't return.
But shouldn't it fails after 10 seconds?

T

@Tockra
Copy link
Author

Tockra commented Jun 30, 2020

Okay the problem seems to be the token.Wait(), which waits for a response by the broker. (this was my fault)
But even when I remove it, the publish needs a strange unregular amount of time .
I built it into a loop which waits 60 seconds between every send.
This is the result:

[net]      incoming stopped with error EOF
[net]      error triggered, stopping
2020/06/30 14:58:37 Lost connection to MQTT Broker:  EOF
[client]   dial tcp ip:8883: i/o timeout
[client]   dial tcp ip:8883: i/o timeout
2020/06/30 14:59:38 Sended
[client]   dial tcp ip:8883: i/o timeout
[client]   dial tcp ip:8883: i/o timeout
2020/06/30 15:00:44 Sended
[client]   dial tcp ip:8883: i/o timeout

It is caused by the Publish itself which needs some time if the connection isn't established. I dont know if that is caused by using a filestore, but why isn't the reconnect process asynchron like the sending process ? So if you want to wait for a reconnect you wait for the token (token.Wait()) ?

@MattBrittan
Copy link
Contributor

Hi @Tockra,

I did notice some issues with this when looking at the code last week (first time I needed to use a write timeout). The writetimeout is not applied to all writes (for instance in Connect or for 'Priority' messages). However I don't think that this is related to your issue.

As you say your initial example is explained by the fact that Token.Wait() will only return when there is an error or the handshake with the broker is completed. However its a bit more complicated than that due to the way AutoReconnect is handled (your token may be completed before the message is actually delivered). This is not ideal but the idea is that if you have QOS of 1+ then message delievery is guaranteed (as per the MQTT standard); it could probably do with a bit of a tidy up but is functional for most use-cases (if you have a specific requirement it might be worth setting that out because changing this will be a bit of work).

why isn't the reconnect process asynchron like the sending process

I'm not sure what you mean by this. If AutoReconnect is set then the package will automatically reconnect when the connection is lost. The Connect() function is async (it returns a token which is only completed when the conenction is up or an error occurs).

The above contains some guess as to why you may be seeing what you are. However as the library has a lot of options it's very hard to assist without a full logs from the library (enable DEBUG logging) and, ideally, a minimal reproducible example.

Matt

@Tockra
Copy link
Author

Tockra commented Jul 1, 2020

I'm not sure what you mean by this.

I mean that the Publish() function needs some time to return when the client isn't connected with the broker. I think it trys some reconnects or something else. But why isn't that asynchron?

And what is the natural way to check if a message was transmitted or not (because of timeout to broker) ? WriteTimeout seems not to has a effect

@MattBrittan
Copy link
Contributor

Apologies; I thought your comment was with regards to the reconnect process (which is async). As you point out Publish waits on the obound channel (which means it will end up waiting on the reconnect). At one point obound was a buffered channel so it would have been async but that is no longer the case (more info in this issue). I agree with you that it is a bit confusing and is an area that could do with some work (and ideally the token should survive a disconnect/reconnect cycle).

And what is the natural way to check if a message was transmitted or not (because of timeout to broker) ? WriteTimeout seems not to has a effect

WriteTimeout does have an effect in that it sets a timeout on the Write that actually transmits the published message (as mentioned there are a few other writes that do not implement the timeout) and causes the Publish command to exit if messages are being queued up (a QOS 1+ message should still be delivered when the reconnection is complete). The issue is that at QOS 1+ writing the message to the network link does not complete the process (at higher QOS levels the message is only considered to have been delivered after a number of messages have been passed back and forth). If you are using QOS 0 then token.Wait() should be reliable but at higher QOS levels the library manages persistence with the aim being that once you send the message you should be able to trust that it will, eventually, be delivered.

While it would be possible to change this to provide confidence that the message has been delivered I'm not sure how beneficial that would be. The reason I say this is that we can only check that the message has been delivered to the broker (and have no information about its delivery to the end consumer(s)).

@Tockra
Copy link
Author

Tockra commented Jul 1, 2020

Okay, so it is intended that publish doesn't return instantly if there is no connection to the broker? Or is it a bug?

@MattBrittan
Copy link
Contributor

Personally I'd call this a bug (because its inconsistent with the way the other calls that return Token's work).

Note that in order to always return instantly we would need to use a buffered channel so there will always be a point at which the return is not instant.

@Tockra
Copy link
Author

Tockra commented Jul 1, 2020

so there will always be a point at which the return is not instant.

Yes of course. But that is clear. But the current behavior of Publish is a little bit strange ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants