Skip to content

Improve session message injection#2117

Merged
chkr1011 merged 11 commits intodotnet:masterfrom
AntonSmolkov:improve_session_message_injection
Jan 2, 2025
Merged

Improve session message injection#2117
chkr1011 merged 11 commits intodotnet:masterfrom
AntonSmolkov:improve_session_message_injection

Conversation

@AntonSmolkov
Copy link
Copy Markdown
Contributor

Hi there!
I'm trying to build a scalable, persistent MQTT server around RabbitMQ, using the MQTTNet server as a frontend.

While doing this, I found that there is currently no proper way to correlate injected QoS 1 messages with the packets received by the ClientAcknowledgedPublishPacketAsync event. So, there is no way for me to pass the PUBACK further to RabbitMQ's message acknowledgment.

Additionally, I noticed that DeliverApplicationMessageAsync() method may hang indefinitely when MqttPendingMessagesOverflowStrategy.DropNewMessage is used, because in the case of an output buffer overflow, the packetBusItem being awaited is dropped from the queue without the Task's Result or Exception being set.

So, in this PR:

  • I added MqttPublishPacket as a result of message injection methods, to be able to correlate them by packet id later.
  • Fixed the potential infinite wait in the DeliverApplicationMessageAsync() method.
  • Added some tests.

Thanks for the great project, btw!

@AntonSmolkov
Copy link
Copy Markdown
Contributor Author

@dotnet-policy-service agree

@AntonSmolkov AntonSmolkov force-pushed the improve_session_message_injection branch from 8d5092d to f96c10c Compare December 1, 2024 14:09
Comment thread Source/MQTTnet.Server/Status/MqttSessionStatus.cs Outdated
Copy link
Copy Markdown
Collaborator

@chkr1011 chkr1011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature! But I recommend changing the public API a bit.

@AntonSmolkov AntonSmolkov force-pushed the improve_session_message_injection branch from 1b4cb4c to 2b1c143 Compare December 2, 2024 16:43
@AntonSmolkov AntonSmolkov force-pushed the improve_session_message_injection branch from 841fa67 to 026029c Compare December 3, 2024 13:40
Comment thread Source/MQTTnet.Server/Status/MqttSessionStatus.cs Outdated
Comment thread Source/MQTTnet.Server/Status/MqttSessionStatus.cs Outdated
Copy link
Copy Markdown
Collaborator

@chkr1011 chkr1011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few minor improvements.

@AntonSmolkov AntonSmolkov requested a review from chkr1011 December 6, 2024 06:48
Copy link
Copy Markdown
Collaborator

@chkr1011 chkr1011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work 👍

@AntonSmolkov
Copy link
Copy Markdown
Contributor Author

@chkr1011 Thanks!

The CI-workflow is not approved yet. It this intentional?

@AntonSmolkov
Copy link
Copy Markdown
Contributor Author

@chkr1011 Sorry to bother you again, but the CI has failed on the "sign nugets step". :))

@chkr1011 chkr1011 merged commit 27771f0 into dotnet:master Jan 2, 2025
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

Successfully merging this pull request may close these issues.

2 participants