moquette-io / moquette

Java MQTT lightweight broker
http://moquette-io.github.io/moquette/
Apache License 2.0
2.27k stars 814 forks source link

Confuse on SessionEventLoopGroup.routeCommand method #813

Closed SpringStudent closed 4 months ago

SpringStudent commented 5 months ago

As you know,routeCommand method would be invoked to handle mqtt request. I am confused about whether the processing order needs to be guaranteed for MQTT protocol requests. If it needs to be ensured, then the following code segment if (Thread.currentThread() == sessionExecutors[targetQueueId]) { SessionEventLoop.executeTask(task); return PostOffice.RouteResult.success(clientId, cmd.completableFuture()); } will result in a probability of unordered request processing. Additionally, considering the places where the routeCommand method is invoked, I can deduce that the above code will not be executed. If it won't be executed, then why is this code segment written?

hylkevds commented 4 months ago

This bit of code happens when the thread that wants to add a command to a queue is the thread that handles the queue the command is added to. This only happens for Acknowledge messages, which should have priority and skip the queue.

SpringStudent commented 4 months ago

This bit of code happens when the thread that wants to add a command to a queue is the thread that handles the queue the command is added to. This only happens for Acknowledge messages, which should have priority and skip the queue.

I cannot find in the source code where the EventLoop thread is used to handle the addition of commands. Therefore, I believe the above code will never be execute,So is there still a need for that code segment?

hylkevds commented 4 months ago

Indeed, this currently also happens for Publish commands if the publisher is on the same thread as the receiver. And this indeed causes these publishes to jump the queue.

There are cases where the command must jump the queue. For instance the SessionRegistry::remove() has the goal of cleaning up the queues. If this happens because the client re-connected with cleanSession=true, this will be called from the same thread, and it should probably happen directly (not entirely sure there).

We may have to add a flag to indicate the action should jump the Queue or not.

SpringStudent commented 4 months ago

be

Indeed, this currently also happens for Publish commands if the publisher is on the same thread as the receiver. And this indeed causes these publishes to jump the queue.

There are cases where the command must jump the queue. For instance the SessionRegistry::remove() has the goal of cleaning up the queues. If this happens because the client re-connected with cleanSession=true, this will be called from the same thread, and it should probably happen directly (not entirely sure there).

We may have to add a flag to indicate the action should jump the Queue or not.

I debugged the source code and realized that you were right. I will close the issue. Thank you for your patient response.