Server forgets non-standard keepalive

Bug #880863 reported by Rob Pridham
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
mosquitto
Fix Released
Undecided
Unassigned

Bug Description

I use the Mosquitto C library as a client, and the Mosquitto server as the broker, both version 0.13.

When I set the client keepalive to a higher than standard number, e.g. 20 minutes, the server decides that the client has disconnected after 90 seconds.

When I set the client keepalive to the default 60 seconds, the client sends PINGRESP before the 90 second period and thus all is well.

I added some output to the server code and it receives the keepalive value OK in mqtt3_handle_connect(). However when read back in mosquitto_main_loop (from the database?) it returns 60 seconds again. I don't have any persistence options configured beyond the defaults.

Edited to add: it appears to me that in read_handle_server.c, around line 200, the database structure is populated with values but the previously set keepalive is missing, i.e. the addition of the line:

db->contexts[i]->core.keepalive = context->core.keepalive;

is required.

description: updated
description: updated
Revision history for this message
Roger Light (roger.light) wrote :

Thanks for a nice bug report.

The code around line 200 in read_handle_server.c is to do with the case where a client connects with the same id as a currently connected client or a disconnected non-clean-session client. I agree that the line you write is missing. Does this completely cover your problem though? If you connect a client just once do you still have the problem?

Revision history for this message
Rob Pridham (rob-pridham) wrote :

Thanks for the swift reply!

To test, I stopped the service, removed the database file (which I only just noticed) and started the service again. On the first connection I see it skip that block and thus use the value supplied in the connection call, which is correct.

In the next connection I see it find the client in the database and retrieve the correct value, so it does appear to end up in the database correctly as part of the first connection.

Therefore as I understand it, what I reported only manifests itself as a problem in the case where a client record is made and subsequently the requested keepalive changes. Incidentally I don't recall ever asking for a keepalive of 60, but my use of the client is still in early development, so it's possible. Alternatively it could be an artifact of my moving from the v0.12 server and not removing the database.

Revision history for this message
Roger Light (roger.light) wrote :

Ah, I think I see what's happening and it should all be fixed with the addition of your line.

The persistent database file is intended to store information across restarts of the broker. That basically boils down to information about clients that have clean_session set to false as well as any retained messages. For the clients this means subscriptions and queued messages. If you have clean_session set to true you shouldn't be seeing them in the persistent database. The broker writes the file every 1800 seconds (as configured in mosquitto.conf) or when it is stopped. It isn't a direct representation of what's in memory though.

Assuming you're using clean_session=false, what will most likely be happening is that you have the broker running and your client connects. If you connect another client with the same id but with a different keepalive value you should see it keep the original keepalive value because of the missing line.

If you now restart the broker, the client details will be written to disk - but this doesn't include keepalive because it's not a persistent piece of data because you have to provide it when you connect. When the broker restarts it loads the stored client information but not the keepalive because it doesn't exist - so the default of 60 seconds is used. When your client connects it is effectively treated as a duplicate client and doesn't get the new keepalive because of the missing line.

If you can confirm that you're using clean_session=false I'll be fairly happy that all of the above is true.

Keeping the db file from 0.12 shouldn't make any difference. The format changed very slightly from 0.12 to 0.13 but 0.13 can read in the older versions.

Revision history for this message
Rob Pridham (rob-pridham) wrote :

Yep, clean_session=false in everything I've done. Your description makes sense.

Revision history for this message
Roger Light (roger.light) wrote :
Changed in mosquitto:
milestone: none → 0.14
status: New → Fix Committed
Changed in mosquitto:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.