Next version TODO list

[shaileshahuja]

Hi,
I have some bandwidth and wondering if there is a TODO list for the next version. I would be happy to contribute.

[MattBrittan]

Thanks - in the discussion here I suggested the following for V1:

• Async publish; notification that message has been delivered (or queued and connection closing) - Receive notification of acknowledgments when using async publish #216
• Manual ACK; consider how reconnections should be dealt with (potentially waiting for handlers to complete before reconnecting)
• Better docs

so work on any of those would be great.

If you want to take a stab at any of those then go for it. A few other ideas:

• RPC - this could do with some major rework (see issues like [RPC-Extension] Configurable response topic #86 autopaho RPC Handler breaks on reconnections #124). I have not actually used this but did look at the code a while back and it's not great.
• General tidy ups - really just read through the source and try to fix any issues/difficult to understand areas.
• Testing - the tests can always be improved!

[shaileshahuja]

@MattBrittan Thanks, trying to understand the following better:

Async publish; notification that message has been delivered (or queued and connection closing)

To clarify, the expectation is to have a callback for Puback, PubRec and Pubcomp? Or do we want to more detailed callbacks?

Tangentially, I am looking at #247 regarding PublishViaQueue, where you mentioned:

Currently there is no way to tell when the message has actually been sent (that's on the to-do list).

I realized that PublishViaQueue has overlapping functionality with SessionManager. If the QOS 1/2 publish to the server fails, it gets added to the session to be retried, so the use-case is perhaps geared towards QoS0 messages? I think if the QOS0 publish fails, we don't need to retry. Is there another reason we want to have PublishViaQueue?

[MattBrittan]

To clarify, the expectation is to have a callback for Puback, PubRec and Pubcomp? Or do we want to more detailed callbacks?

Personally I feel that a single 'PublishComplete' type callback would be sufficient for most users. However it is possible that some QOS2 users will want to know when the PUBREL has been recieved (might, for example, commit a database transaction at this point). I think Callbacks are probably the default approach for this kind of thing (the main alternative being a token type approach as used in the V3 client, that was not ideal).

If the QOS 1/2 publish to the server fails, it gets added to the session to be retried

Unfortunately it's a bit more complex than that. The connection may be down at the time Publish is called, or the server Receive Maximum setting mean that we need to wait for previous packets to be acknowledged before sending this one. In addition the Packet Identifier is two bytes so a maximum of 65535 packets in the session (and this may be an issue if a client is offline for a long time). For these reasons (and others, such as packet ordering, resending upon reconnection etc) I felt that seperating the queue and session was the best approach.

[shaileshahuja]

Personally I feel that a single 'PublishComplete' type callback would be sufficient for most users. However, it is possible that some QOS2 users will want to know when the PUBREL has been received

What's your opinion on having two callbacks:

• For most users, a basic onPublishSuccess callback, that is called on PUBACK for QoS1, and PUBCOMP for QoS2
• For advanced users, an onPacketReceived callback would expose all received packets, including PINGRESP, SUBACK, PUBREL, etc.

Currently there is no way to tell when the message has actually been sent (that's on the to-do list).

Understood on session/queue separation. To double-check, would you still want a callback when PublishViaQueue sends the message?

[MattBrittan]

What's your opinion on having two callbacks:
* For most users, a basic onPublishSuccess callback, that is called on PUBACK for QoS1, and PUBCOMP for QoS2
* For advanced users, an onPacketReceived callback would expose all received packets, including PINGRESP, SUBACK, PUBREL, etc.

I wonder if onPublishComplete() might be better, this would be called when the operation completed regardless of successs or failure (important that we consider that delivery may fail; the user might have a goroutine waiting on completion to perform some follow up action).

My thought was that the function would be passed into the Publish call (so it could be different for each message); the reasoning behind that is that otherwise the user would need to work out what message the callback relates to and that could be difficult (the PacketID is hidden from the user, and not a good indicator anyway as it can be reused). The same would apply to the onPacketReceived (meaning PINGRESP would not be handled by this call).

I did consider other options (e.g. using an Iterator to loop through the packets received) but that would force the caller to start a goroutine for each message so did not seem like a great approach. Not really a fan of callbacks but they seem the best option here.

One thing that is a bit trickey here is what to do when the connection is shutdown:

• Reconnects are fine but the onPublishComplete() (or whatever) function would need to be called if any messages are not going to be delivered due to loss of the session state.
• On shutdown the user probably needs to be advised that there is a chance the messages will be sent whan it's next started (don't really think there is good way of handling this).

Currently there is no way to tell when the message has actually been sent (that's on the to-do list).
Understood on session/queue separation. To double-check, would you still want a callback when PublishViaQueue sends the message?

Yes - one option would be if onPacketReceived is just onPacket() and we call it with the relevant PUBLISH packet (perhaps with the option for the user to choose to abort and not send the packet - but that might be overcomplicating things).

[shaileshahuja]

I wonder if onPublishComplete() might be better, this would be called when the operation completed regardless of successs or failure

I agree onPublishComplete() is better than onPublishSuccess(). However, I am worried we are introducing new terminology/keywords not part of the protocol definition, and maybe creating confusion about what `complete' means in this case.

My thought was that the function would be passed into the Publish call (so it could be different for each message)

How would we maintain this mapping, are you thinking it's a separate in-memory data structure? Will it shoot the memory usage too much? For reference, the Python paho client provides global callbacks per client for each packet type and exposes the packet ID to the user. This avoids the library having to maintain the callback mapping to the message.

For some users, the convenience of not having to keep track of packet ID and match it with incoming packets might be quite useful. But I think for most users, having global callbacks should be sufficient, they don't result in increased memory usage and don't have complications due to restarts as you have noted. To start with, maybe we can do global callbacks first? Or any other thoughts on this?

Not really a fan of callbacks but they seem the best option here.

I think the callbacks should be started as goroutines to ensure we can continue processing messages, in case the user provided function is slow or buggy, we don't want to block the network processing.

Reconnects are fine but the onPublishComplete() (or whatever) function would need to be called if any messages are not going to be delivered due to loss of the session state.

I didn't follow this, wouldn't the session state persist across the reconnects?

[MattBrittan]

I agree onPublishComplete() is better than onPublishSuccess(). However, I am worried we are introducing new terminology/keywords not part of the protocol definition, and maybe creating confusion about what `complete' means in this case.

I prefer this approach because it's consistent across all QOS levels, and is what most users are likely to be after (a lot of users will not understand the various packets involved) - this is consistent with the C client (which uses deliveryComplete).

How would we maintain this mapping, are you thinking it's a separate in-memory data structure?

This will, at minimum, be a function pointer, so generally 64 bits. For the vast majority of users 1-2 would be in memory at a time, for high volume publishers this may be significantly higher but I suspect it would be rare to have more than 65535 messages queued (the limit of packet IDs) so that would be 256kb RAM (not really significant). This probably understates this a bit as the functions will generally have some context but I think demonstrates that it's workable.

For reference, the Python paho client provides global callbacks per client for each packet type and exposes the packet ID to the user.

Correct - but the Pythin Paho client immediately issues a message identifier to the message, and returns that to the user - this library does not (the Go V3 client did use that approach) when using a queue. Issuing ID's to messages immediately imposes some limits (i.e. 65535 messages queued) and I believe complicates things (you need additional flags to indicate if the message has actually been sent, and the queue code becomes tightly coupled to the session code).

I think the callbacks should be started as goroutines

My preference would be to allow the caller to make this choice (i.e. don't start a goroutine in the library). Starting a GoRoutine within the library means that the order in which the callbacks run is not guaranteed, and means that syncronisation (Mutex etc) will be needed (which may be possible to avoid if run in sequence).

I didn't follow this, wouldn't the session state persist across the reconnects?

It can, but that depends upon the connection and broker settings. If there is a low Session Expiry Interval then the session may be cleared off the server before the client reconnects (or the broker may be restarted and not have the session stored to disk etc). See things like this; it's important that messages don't get stuck (so we guarantee that if a callback is configured then it will be called).

[shaileshahuja]

Understood, I made a start here to get your thoughts on the direction.

• Re-using PublishResponse struct for both async and sync calls
• Re-using PublishOptions to add the callback

[shaileshahuja]

I made another PR to get your thoughts on adding this callback to autopaho