eclipse / paho.golang

Go libraries
Other
330 stars 92 forks source link

Redundant calls in message handlers with StandardRouter #204

Closed cloudxxx8 closed 9 months ago

cloudxxx8 commented 9 months ago

Describe the bug If we want to subscribe multiple topics with wildcard and set up different message handlers for them, the message handlers

To reproduce There are two topics to be subscribed and tow message handler function for each of them:

Then, publish a message to the topic testing

Debug output In the console output, we can see the handler1 and handler2 are called twice

This is handler1 This is handler2 This is handler1 This is handler2

Expected behaviour We expect they should be called once

This is handler1 This is handler2

Software used:

Additional context This issue happens because the Subscribe and Router are separated. Although this is a powerful design, is there any approach to avoid this situation? Users may not want the duplicated messages received from the same client.

In the MQTT v3 paho lib, there is no this kind of problem. Sample code in v3:

package main

import (
    "fmt"
    "time"
)
import pahoMqtt "github.com/eclipse/paho.mqtt.golang"

func main() {
    options := pahoMqtt.NewClientOptions().AddBroker("localhost:1883")
    c := pahoMqtt.NewClient(options)
    if token := c.Connect(); token.Wait() && token.Error() != nil {
        panic(token.Error())
    }

    token := c.Subscribe("testing", 0, func(client pahoMqtt.Client, message pahoMqtt.Message) {
        fmt.Println("This is handler1")
    })
    token.Wait()
    if token.Error() != nil {
        panic(token.Error())
    }

    token = c.Subscribe("testing/#", 0, func(client pahoMqtt.Client, message pahoMqtt.Message) {
        fmt.Println("This is handler2")
    })
    token.Wait()
    if token.Error() != nil {
        panic(token.Error())
    }

    fmt.Println("Start sleeping")
    time.Sleep(10 * time.Second)
    c.Disconnect(0)
    fmt.Println("Disconnected")
}
MattBrittan commented 9 months ago

Have taken a look and this looks like it's Mosquitto related (testing using Mosquitto 2.0.17):

With the V5 test:

docker-mosquitto-1  | 2023-11-19T08:44:00: Received SUBSCRIBE from PahoGoClient
docker-mosquitto-1  | 2023-11-19T08:44:00:      testing (QoS 0)
docker-mosquitto-1  | 2023-11-19T08:44:00: PahoGoClient 0 testing
docker-mosquitto-1  | 2023-11-19T08:44:00:      testing/# (QoS 0)
docker-mosquitto-1  | 2023-11-19T08:44:00: PahoGoClient 0 testing/#
docker-mosquitto-1  | 2023-11-19T08:44:00: Sending SUBACK to PahoGoClient

docker-mosquitto-1  | 2023-11-19T08:44:03: Received PUBLISH from auto-8754B1D8-7A58-0E00-F746-7AC1DC2D97DE (d0, q0, r0, m0, 'testing', ... (3 bytes))
docker-mosquitto-1  | 2023-11-19T08:44:03: Sending PUBLISH to PahoGoClient (d0, q0, r0, m0, 'testing', ... (3 bytes))
docker-mosquitto-1  | 2023-11-19T08:44:03: Sending PUBLISH to PahoGoClient (d0, q0, r0, m0, 'testing', ... (3 bytes))
docker-mosquitto-1  | 2023-11-19T08:44:03: Received DISCONNECT from auto-8754B1D8-7A58-0E00-F746-7AC1DC2D97DE

However with the V3 test:

docker-mosquitto-1  | 2023-11-19T08:46:06: Received SUBSCRIBE from auto-2BA33DDE-DBD1-802E-8987-251B7F0C6FE5
docker-mosquitto-1  | 2023-11-19T08:46:06:      testing (QoS 0)
docker-mosquitto-1  | 2023-11-19T08:46:06: auto-2BA33DDE-DBD1-802E-8987-251B7F0C6FE5 0 testing
docker-mosquitto-1  | 2023-11-19T08:46:06: Sending SUBACK to auto-2BA33DDE-DBD1-802E-8987-251B7F0C6FE5
docker-mosquitto-1  | 2023-11-19T08:46:06: Received SUBSCRIBE from auto-2BA33DDE-DBD1-802E-8987-251B7F0C6FE5
docker-mosquitto-1  | 2023-11-19T08:46:06:      testing/# (QoS 0)
docker-mosquitto-1  | 2023-11-19T08:46:06: auto-2BA33DDE-DBD1-802E-8987-251B7F0C6FE5 0 testing/#
docker-mosquitto-1  | 2023-11-19T08:46:06: Sending SUBACK to auto-2BA33DDE-DBD1-802E-8987-251B7F0C6FE5

docker-mosquitto-1  | 2023-11-19T08:46:11: Sending CONNACK to auto-07E8E4AC-7F2D-9F34-5968-BE0FCE264DBB (0, 0)
docker-mosquitto-1  | 2023-11-19T08:46:11: Received PUBLISH from auto-07E8E4AC-7F2D-9F34-5968-BE0FCE264DBB (d0, q0, r0, m0, 'testing', ... (3 bytes))
docker-mosquitto-1  | 2023-11-19T08:46:11: Sending PUBLISH to auto-2BA33DDE-DBD1-802E-8987-251B7F0C6FE5 (d0, q0, r0, m0, 'testing', ... (3 bytes))
docker-mosquitto-1  | 2023-11-19T08:46:11: Received DISCONNECT from auto-07E8E4AC-7F2D-9F34-5968-BE0FCE264DBB

Mosquitto is sending the PUBLISH once with V3 but twice with V5; so this is a difference in how Mosquitto handles V3 vs V5 subscriptions where there are overlaps. I believe the behaviour is explained in this issue.

As such I don't believe there is really a problem in this library to resolve. The router behaviour is pretty similar between the V3 and V5 clients; the fact that V3 automatically adds a route when Subscribe is called is not really relevant (one of the reasons we don't do this is because it becomes confusing at QOS1+ with persistent sessions, because the subscriptions remain active and you can receive messages immediately after connection).

I guess you could use the SubscriptionIdentifier to uniquely identify a particular subscription (the broker should then pass this with the PUBLISH).

cloudxxx8 commented 9 months ago

@MattBrittan thanks for the quick response. I believe there is nothing to do in this library, closing this issue.