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

Receive notification of acknowledgments when using async publish #216

Open
MattBrittan opened this issue Dec 29, 2023 · 9 comments
Open
Milestone

Comments

@MattBrittan
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When using async PUBLISH (i.e. PublishWithOptions with PublishMethod_AsyncSend) it's not currently possible to determine when the message has been acknowledged (with QOS 2 messages it would be useful to know when both steps are complete).

Describe the solution you'd like
This was discussed a bit in issue #207 options include:

One way to achieve this is to change AddToSession() in SessionManager to return a function that will block until the ack is received. The paho client would just return that function to the user. The function would be valid for use for as long as the session is.

My thought on this was to add callbacks to PublishOptions; we could then store the PublishOptions in state.clientPackets (making it easy to call when an associated event occurs). I prefer the callback approach because I think it's easier for the end user (and avoids the need to have a lot of go-routines hanging around waiting for acknowledgements). One note of caution is that the session may outlast the client instance (and I can't think of a good way of handling that!).

@MattBrittan
Copy link
Contributor Author

Carrying on discussion from #207 comment by @vishnureddy17

One note of caution is that the session may outlast the client instance (and I can't think of a good way of handling that!).

I think that's expected and necessary. As a user, I'd want to know when the publish was handled in the session (regardless of how many reconnections happened in the interim). In the solution you suggest, that would mean that the callback can potentially be called long after a particular client is dead.

Just to clarify; by "client instance" I meant the users program (rather than an instance of paho/autopaho). As the session can be stored on disk it's quite possible for the following to happen:

  1. User publishes message and it's added into session (or queue in autopaho)
  2. Client application restarts (say a server restart or application update)
  3. Message actually sent

This raises a few questions:

  • Is the callback called when the application is shutting down (only possible on a clean shutdown obviously)?
  • Does the library provide a way to 'reregister' the callbacks (or whatever mechanism is used) when starting up?

My thought was always that if autopaho is in use then whatever notification method is selected should survive reconnections (these should be mostly hidden from the end user). We do also need to handle the situation where the session is lost (either due to Clean Start or the session expiring) so probably want to be able to pass an error to the user when this happens.

@vishnureddy17
Copy link
Contributor

Does the library provide a way to 'reregister' the callbacks (or whatever mechanism is used) when starting up?

This is an interesting issue. Currently, the SessionManager interface doesn't expose a way to get the un-acknowledged messages that are in the session. So on an application restart, the application has no way of knowing which messages to re-register callbacks for. In other words, the application doesn't have knowledge of the messages that are pending. A similar problem exists for Queue in autopaho.

Another issue is that even if the library provided a way to re-register callbacks, the user would have to know to register it before a connection is established since messages get resent in ConAckReceived.

One possible solution is to have a callback for "orphaned" messages. However, this callback may be of little use since the callback is for publishes that the application has lost track of. This could perhaps be mitigated by including the re-sent packets as part of arguments supplied to the callback.

I think the problem of application restarts does not need to be solved immedidately. The solution for non-restarting applications is clearer, so it may be a better idea to solve that problem first.

My thought was always that if autopaho is in use then whatever notification method is selected should survive reconnections (these should be mostly hidden from the end user). We do also need to handle the situation where the session is lost (either due to Clean Start or the session expiring) so probably want to be able to pass an error to the user when this happens.

Agree with this.

@MattBrittan
Copy link
Contributor Author

Have been thinking about this following PR #264 (not yet accepted). The issue here is that packets are stored (to memory/disk) in two places (queue and session store), so in order to notify the client that a message has been sent we need to store some kind of ID so that the specific message can be identified and tracked through the queue/store. @shaileshahuja has had a go at this for the queue by adding a UUID that's maintained in a seperate map (or part of the filename fot the filestore). This would work but requires a range of changes and I had some concerns about requiring a UUID.

I wonder if an alternative option is to make it the users responsibility to provide the ID and:

  • Have the user add a property (i.e. 0x26 User Property with a known identifier (i.e. PahoID)
  • Add callbacks that notify the user that the ID has passed various milestones (i.e. In session, transmitted, complete)
  • For QOS2 copy the property into the PUBREL packet before sehdning this to the session store (so it's available when the PUBCOMP is received)
  • Remove the property just before the message is transmitted (potentially optional).

This would simplify the changes required (would be minimal) whilst providing the notification required. It would also enable the authors of stores (e.g. a databased queue) to extract the ID into a different database field if they want (parse the packet). It's definitly not perfect but I think it's a workable solution so thought I'd put it out there to see what others think.

@thedevop
Copy link
Contributor

We can take a similar approach as Sarama (Kafka client library), where there is a Metadata field available for the user program.

	// This field is used to hold arbitrary data you wish to include so it
	// will be available when receiving on the Successes and Errors channels.
	// Sarama completely ignores this field and is only to be used for
	// pass-through data.

This would be part of the Publish (and others) packet (but would not hit the wire), we can use []bytes instead of interface to avoid complication storing on disk.

@MattBrittan
Copy link
Contributor Author

Thanks @thedevop - what I'm proposing is slightly different from the approach Sarama takes in that I'm suggesting we use an MQTT v5 property for this (so it's part of a valid packet, that could be sent to the server, as opposed to a field that's purerly local). The property would, optionally, be removed prior to the request hitting the wire (some use-cases might have an existing property with a unique identifier that is sent as a property already so it would make sense to allow that to be utiised).

The benefit of this approach is that it means the existing queue and store already provide the needed functionality (and continue to store "raw" MQTT packets). This also means that the identifier will survive an application restart when a disk based queue/store is used (so an app can publish a message, be restarted, then receive notification that the message has been successfully transmitted).

A potential disadvantage is that we would need to read/decode the packet from the store when an acknowledgment is received (but it would be possible for the store to have extended functionality to deal with this if it becomes an issue).

@thedevop
Copy link
Contributor

Do you think it's a concern that a user property key will be reserved and can not be used by user program?

@MattBrittan
Copy link
Contributor Author

Do you think it's a concern that a user property key will be reserved and can not be used by user program?

My thought is that we allow the user to select the key (so any conflicts could be avoided). In fact, it probably makes sense that the user sets the property too.

The changes needed to this package would be:

  • copy it across to qos2 pubrel packet
  • remove before sending out on the wire)
  • callback when message fully delivered (and, perhaps, at other stages)

Need to prototype this to see if it works.

@thedevop
Copy link
Contributor

If the user can select the key, wouldn't that require which key the user selected (purely local) also be persisted? That's in addition to the key/value in the User Property.

@MattBrittan
Copy link
Contributor Author

If the user can select the key, wouldn't that require which key the user selected (purely local) also be persisted?

If would just be a ClientConfig option (obviouslly an app would need to stick with the same value).

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