Data Value References?
jkoplo opened this issue · 6 comments
There's some strange intricacies because of the "Object by Value" implementation of LVClasses. With the MQTT Client class, pretty much all configuration/private data is only set once at initialization. The exception to this is the Session which gets created/set on Connect.
Where I'm running into an issue is when I need to handle reconnecting. I'm monitoring the Client connection in a separate loop than the one that I'm receiving/sending in. When I see the connection fail, I call a Connect but this overwrites the Session and the other loop still has the old version.
I see three possible solutions:
- I refactor so all my handling of connect/disconnect/reconnect is one place
- Turn the
Sessionprivate into a Data Value Reference of Session - In the
ConnectVI, check if the session is already valid and merely reconnect it vs generating a new session
I think number three is just going to push the problem down into the Session class where it will be generating a new reference, unfortunately. I've handled similar things with the Data Value Reference solution - that makes it behave more like a class in a standard OOP language. I can also looking into the refactoring - if I recall all the publish/subscribe stuff is handled with notifiers that don't get recreated, so it's probably independent of the Session value... maybe.
Thoughts?
I usually code with as much "by Value" as I can, so my first inclination would be to recommend to send messages to a single loop, which would serialize the call made to the Client class, which essentially boils down to your solution 1.
Solution 3 is not ok because you would probably need an extra mechanism to prevent a new session being created while you are shutting down... A parallel reconnecting loop can cause a race condition with the shutdown process. You'd have to make sure it does not happen. That's generally the reason I like to stay within a single loop for that kind of things. The loop is the state machine, and it knows that a "reconnect" message needs to be rejected if the state is "shutting down". In this case, I usually wrap the client code into an Actor (AF or SMO).
As for solution 2, that seems a simple fix to achieve your goal and I will definitely look into doing that. Not that it wouldn't work, but I think there will need to be a bunch of verifications and certainly a list of new standard error codes to come up with to enable such a refactor.
On the surface, it seems as simple as storing the Session in a DVR. But in reality, there will be a risk of multiple loops trying to publish, subscribe, connect and disconnect in parallel. Not that I'm advocating strongly against that. It also impacts the other members of the private data for the client. We'd have to answer the question such as : what if the user calls the "Stop" method? Should it work in parallel as well? Now, I'm not a big fan of that at all.
For the moment, I can suggest solution 4 to get you going: wrap the client into a by-Reference object... You can create your own by-ref class on which you can definitely fork the wire as you want and have a parallel reconnecting loop.
Personally, I prefer this simple pattern:
Yep, my preference would be to do keep it all by value in a shift register, but the situation makes it challenging.
In my case, I've got cRIOs which are each controlling several independent tests. Each test runs it's own state machine (cloned) and can be started/stopped/etc independently. We're migrating to MQTT for these tests to receive commands and report their data.
The semi-obvious solution would be for each test to instantiate it's own MQTT client/connection, but for cloud platforms and mutual auth that would mean dealing with multiple sets of certs for every tester. It's also extra overhead in terms of more TCP connections.
On the receiving side, I already receive the messages and 'fan them out' to the multiple state machines. Maybe the easiest solution is to do something like that in reverse - have the state machines stick messages in a separate 'outbound' queue and let my single loop that handles the inbound messages also send the outbound. I'd lose some error handling in my individual state machines, but I probably want to suppress network errors anyway - it's valid for the tests to run disconnected.
I like the other solution you proposed too - wrapping the actual client in a DVR. The DVR will handle the mutex as long as I'm careful about keeping it open for my full transaction. That should be fine, since I think I only need it for Publish at this point.
Some good ideas. I'll have to think about it decide which path I take. Long-term, I think it might be common for people to want to access the MQTT client in both loops of a Producer/Consumer - receiving messages in the Producer and sending them in the Consumer.
OK, I understand the use case better now. I'll be on the lookout for solutions. I still need to wrap my brain around the corner cases that will arise by going with a DVR...
By the way, I'm sure you see some familiarity between the top and bottom code:
note: the bottom loop is the JKI State Machine Objects framework base template.
Since I've been heavily involved in the development of the State Machine Objects, this is my go-to template for any wrapper.
(for other people reading this thread: Solution 1 is the top VI, and solution 4 is the bottom one, where the client is wrapped into an Actor which allows forking the wire and where the client code runs serially inside the process/actor core).
@jkoplo
I did a more thorough review and there didn't seem to be any negative impact to your proposal, although I didn't use the DVR to provide the mutex because the calls are not instantaneous. I didn't want any race conditions or to do any session requests inside the IPE, so there is an optional mutex flag on the Create Client method (defaults to False).
Thanks for the feature requests. I think this is great additions to the client.
New palette:
Try it out. https://github.com/LabVIEW-Open-Source/MQTT-Client/releases/tag/3.1.4
I'll push it to VIPM community shortly as well.
I tested it and it worked well. This is from the stress test in the Client's unit tests:
This is the result of using a server and two clients together with every combination of connect-disconnect it entailed:
Implemented and released in 3.1.4
Thanks for all the quick mods. I'll have to upgrade and test in my application.