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

At reconnect, the client doesn't seem to take max_inflight_messages into account #492

Closed
LOorts-Aloxy opened this issue May 20, 2020 · 1 comment · May be fixed by #698
Closed

At reconnect, the client doesn't seem to take max_inflight_messages into account #492

LOorts-Aloxy opened this issue May 20, 2020 · 1 comment · May be fixed by #698
Labels
Status: Available No one has claimed responsibility for resolving this issue. Status: More info needed More information needed from issue author

Comments

@LOorts-Aloxy
Copy link

LOorts-Aloxy commented May 20, 2020

Hi,
We've been using your service for some time now and are very happy about the results. Thanks for this!

Our setup is the following:
Sensor - (Dash 7)-> gateway -(mqtt)-> server
On the server, we do authentication of the gateway but also the sensor sending the information. When an unauthenticated sensor sends a message through our gateway, the gateway gets disconnected from the broker as per the following documentation: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc39871804 .

Now I've created some code to manually delete the last entry and this seems to work but when the service reconnects, it seems to publish all messages in the _out_messages queue while we've set the max_inflight_messages to 1. I've looked at the code and looks like there's no check for this (https://github.com/eclipse/paho.mqtt.python/blob/master/src/paho/mqtt/client.py#L3054). Shouldn't we check here if we're not publishing to many messages at once?

Thank you in advance!

Kind regards,
Liam

@LOorts-Aloxy LOorts-Aloxy changed the title Only delete last message from queue at disconnect At reconnect, the client doesn't seem to take max_inflight_messages into account May 20, 2020
Juha-Ylikoski-Treon added a commit to Juha-Ylikoski-Treon/paho.mqtt.python that referenced this issue Dec 30, 2022
Previously if the message queue filled up for some reason like bad
connection and the client disconnected (due to e.g. bad connection),
it would send all of it's messages in _out_messages-queue and not
respect _max_inflight_messages parameter. With some brokers and
when we had 1000s of messages in queue and all being sent with
qos 1 without any regard to the _max_inflight_messages, the broker
would disconnect us or the connection would again break down.

This patch makes sure _max_inflight_messages is respected when
reconnect happens for any reason.

I believe this also fixes eclipse#492

Signed-off-by: Juha Ylikoski <juha.ylikoski@treon.fi>
Juha-Ylikoski-Treon added a commit to Juha-Ylikoski-Treon/paho.mqtt.python that referenced this issue Dec 30, 2022
Previously if the message queue filled up for some reason like bad
connection and the client disconnected (due to e.g. bad connection),
it would send all of it's messages in _out_messages-queue and not
respect _max_inflight_messages parameter. With some brokers
when we had 1000s of messages in queue and all being sent with
qos 1 without any regard to the _max_inflight_messages, the broker
would disconnect us or the connection would again break down.

This patch makes sure _max_inflight_messages is respected when
reconnect happens for any reason.

I believe this also fixes eclipse#492

Signed-off-by: Juha Ylikoski <juha.ylikoski@treon.fi>
@MattBrittan
Copy link
Contributor

Apologies for the huge delay in responding (we are working through old issues currently).

Can you please confirm if this is still an issue for you with the current release? I would also appreciate it if you could update the question as the links are now outdated making the issue a little unclear. It would also be useful to see the code you created to "manually delete the last entry".

Note:; If we do not hear from you in a month we will assume this is no longer an issue (not ideal but the only option with issues as old as this).

@MattBrittan MattBrittan added Status: Available No one has claimed responsibility for resolving this issue. Status: More info needed More information needed from issue author labels Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Available No one has claimed responsibility for resolving this issue. Status: More info needed More information needed from issue author
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants