Thanks for the feedback Łukasz and Sebastian, I think you've convinced me that having an opcua specific class to handle the allocation of transactionIds would be a good thing. opcua is possibly unique in that all transactionids need to be received by the server in sequential order, otherwise it will close the connection. At the moment I'm only seeing this issue when I'm closing a connection due to some lazy programming, but as I have seperate threads for each group of subscriptions I am expecting it to cause more issues as I go through a few more tests.
I wanted to avoid having a mutex on the allocation of ids as there could be a relatively long time a thread is locked out where it could be encrypting a message. I think I might end up having a seperate class that allocates ids freely but has a seperate canSubmitTransactionWithId method that returns an optional future that gets completed when it is its turn to submit the transaction. But i'll sleep on it before implementing anything. Łukasz, yeah I do have to take out the transaction manager from the Subscriptionhandle class and use the Protocollogic class's manager (As well as actually use it for half the protocollogic messages), but I think that is mainly to be able to limit the number of messages I have in flight. Thanks again for your help :) Ben On Mon, May 3, 2021 at 5:18 PM Łukasz Dywicki <[email protected]> wrote: > Looking at the opcua code right now: > > https://github.com/apache/plc4x/blob/ee597d20c25f13203a43adf0e5f3d0b4e189c4c4/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/OpcuaSubscriptionHandle.java#L150 > > The transaction id should be generated inside runnable passed to > transaction.submit() method call. This guarantees that your tx id will > always be most up to date. > Also, as far I know RequestTranactionManager should be managed at > protocol logic level as it is able to coordinate parallel access to > client socket. Local usage as in above example allows you to coordinate > just this particular subscriber instance. Second subscriber will have > its own RTM which will coordinate his own work log. > > Best, > Łukasz > > On 03.05.2021 18:19, Sebastian Rühl wrote: > > Hi Ben, > > > > answers inline: > > > >> - Change the RequestTransactionManager class so that it processes the > >> requests in the transaction ID order instead of the submitted order. > > This wouldn’t solve the problem because how would the manager know that > there is a latter message inbound with a lower transaction id. (I mean you > could loop/sleep for a required id but that sound inefficient) > >> - Create a class that retains the transaction id order. > > Same problem as above. Its not really about the order > >> - Modify the RequestTransactionManager to have the option between the > >> two. > > As both options don’t solve the issue from my understanding you need > something different. > >> > >> I'd prefer modifying the existing class to use the transaction id order, > >> but don't know enough about the other protocols PLC4X supports to say > that > >> it won't affect them. > >> > >> Kind Regards > >> > >> Ben > > > > For me it sounds a bit like you would need to get control over over your > threads and/or use a pooled connection. > > Root cause is that the creation of the transaction and the scheduling is > decoupled and here you get the race condition. Another way to solve this is > to use a mutex that gets opened when a transaction is opened and closed > when the message is queued. As you said this is related to OPCUA its fine > to add this to the protocol logic there. > > > > Sebastian > > >
