Is it bad practice to block inside of addDataChangeListener?

[Marax]

Hi I have a code (kotlin) as follows and in one of the instances it stopped working for me. I had a lot of readValues calls inside there and
because of them whole call took minutes to resolve. But after all read calls finished, I didn't get any further changeListener calls even though the value in nodeAddress kept changing. Just a note, value in OPC changes only after last writeValue in changeListener, so no concurrent changes should be triggered. Sampling interval was 200ms.

Right now I've tried to run content of changeListener in new thread and it resolved the issue, so my question is if it's a good practice to run it in thread (executor) always? Also I still suspect that it could also be some kind of overload of that SIEMENS PLC. Also I use same client for multiple other calls and it seemed that this dataChangeListener blocked all other?

val subscription = ManagedSubscription.create(client, samplingInterval)
subscription.addDataChangeListener { items, values ->
  items.forEachIndexed { index, _ ->
    //possibly lot of client.readValues() and other blocking stuff
    //finally writeValue which will trigger opc to change DataItem again
  }
}
subscription.createDataItem(
  samplingInterval,
  ReadValueId(
	NodeId.parse(nodeAddress),
	AttributeId.Value.uid(),
	null,
	QualifiedName.NULL_VALUE
  )
) { .... }

[kevinherron]

Yes, what you are really doing is adding a ChangeListener, and if you read the docs for ChangeListener::onDataReceived it mentions that you shouldn't block in the callback.

milo/opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/subscriptions/ManagedSubscription.java

Lines 1118 to 1128 in c6a875c

	/**
	* New values for {@code dataItems} have arrived.
	* <p>
	* Take care not to block unnecessarily in this callback because subscription notifications are processed
	* synchronously as a backpressure mechanism. Blocking inside this callback will prevent subsequent
	* notifications from being processed and new PublishRequests from being sent.
	*
	* @param dataItems the list of {@link ManagedDataItem}s for which new values have arrived.
	* @param dataValues the corresponding {@link DataValue} for each {@link ManagedDataItem} in {@code dataItems}.
	*/
	default void onDataReceived(List<ManagedDataItem> dataItems, List<DataValue> dataValues) {}

[Marax]

Thanks, was searching around, but didn't see this.

But will two subscriptions affect each other? When I have one sub on client which I block, all other subs will be blocked too?

[kevinherron]

Yes.

edit: yes and no, I guess. It can interfere with other Subscriptions because it prevents the next PublishRequest from being sent, but the "notification semaphore" being used is per-Subscription.

edit2: scratch that, there is a delivery queue, it will affect other Subscriptions.

[Marax]

Ok, I see. Thanks.

Could making delivery queue (ExectutionQueue) concurrencyLimit configurable solve this issue or it would potentionally create other issues?

[kevinherron]

No, the queue is used to ensure ordering. If you introduced concurrency it would be possible for your application to receive or process value changes out of order.

You need to stop blocking on this thread. Move to another thread to do your blocking work.