opcua icon indicating copy to clipboard operation
opcua copied to clipboard

Add SetReadDeadline for Conn

Open cckolya opened this issue 1 year ago • 13 comments

Hi!

https://github.com/gopcua/opcua/blob/6fd3debc3ef8c234e8524be2d2bdd71e62107d11/uacp/conn.go#L358 https://github.com/gopcua/opcua/blob/6fd3debc3ef8c234e8524be2d2bdd71e62107d11/uacp/conn.go#L373

This code snippet blocks a running program while network is down. I suggest adding something like this before io.ReadFull()

c.SetReadDeadline(time.Now().Add(time.Second * c.readDeadline))

This will solve the following problems:

  1. After the network is disconnected for about 20 seconds, the program is blocked for 3 minutes (this is the default value of net. Conn timeout)
  2. The application understands after 3 minutes when the OPC server is down or the connection between the application and the opc server is broken

How do I reproduce my problems?

  1. Subscribe to one or more OPC server tags.
  2. Just turn off the network for about 20 seconds

Example how I create opcua.Client:

opts := make([]opcua.Option, 0)
opts = append(opts, opcua.RequestTimeout(time.Second*5))
opts = append(opts, opcua.DialTimeout(time.Second*5))

opcua.NewClient(settings.Url, opts...)

cckolya avatar May 04 '23 13:05 cckolya

Seems reasonable. Can you make a PR?

kung-foo avatar May 11 '23 08:05 kung-foo

Though this would need to be deconflicted with the calls to SetDeadline that are based on the incoming context.

kung-foo avatar May 11 '23 08:05 kung-foo

@kung-foo what would a cleaner solution look like? We can break the API with v0.5 if it makes sense since we're going to anyway.

magiconair avatar May 11 '23 11:05 magiconair

Also, would all calls require the same ReadDeadline? What about a PublishRequest? That will wait for a long time

magiconair avatar May 11 '23 11:05 magiconair

Yeah, was thinking later about some of the other pub/sub style calls. I'll need to read more into how keep-alives are implemented (i.e. TCP vs OPC-UA keep alive message). Just a few weeks ago we ran into an issue where Aveva would do SYN,SYNACK,ACK and then nothing. Using https://github.com/gopcua/opcua/pull/629 "fixed" it.

kung-foo avatar May 11 '23 12:05 kung-foo

At minimum, every call that takes a context should apply a deadline (if the context has one).

kung-foo avatar May 11 '23 12:05 kung-foo

I found this PR #628 will fix this issue

cckolya avatar May 15 '23 06:05 cckolya

@Kolyan4ik99 can you test what the read deadline does with PublishRequests ?

magiconair avatar Jun 13 '23 16:06 magiconair

I'm a bit reluctant to merge #628 without knowing that.

magiconair avatar Jun 13 '23 16:06 magiconair

If code c. readTimeout == 1 or c. readTimeout == time.Now().Add(1) for example, then n, err := c.Read(b[:hdrlen]) instantly return err that time is expired

I am using this change in a project and it creates some new problems. My OPC server updates parameters by subscription once every 2 minutes, and I set readTimeout = time.Now().Add(time.Minute):

  • If the OPC server does not return the parameters on the subscription until the readTimeout has expired, then the subscription is closed and I must re-subscribe. Or I must set readTimeout = the interval of the most frequently updated parameter

I need to know the interval of the most frequently updated parameter or use a re-subcribiction

There were no other problems

cckolya avatar Jun 13 '23 16:06 cckolya

~~So this means that the read timeout depends on the type of request? For anything but the PublishRequest we can use the configured value but for PublishRequest we don't set it at all or make it dependent on the subscriptions? Note that the OPC server only sends updates if values actually change. There is no guarantee that you'll get a response AFAIU.~~ Ignore this because it is irrelevant.

The uacp.Conn#Receive method reads the next message from the stream. If the server does not send any message within the ReadDeadline time limit we get an os.ErrDeadlineExceeded error. The question is how to interpret this.

The secure channel dispatcher runs in the background always receiving messages from the server. It does not and cannot know if and when the server sends a message since there are no periodic keep alive messages coming from the server.

It is valid for the OPC server to not send data. I can just open a secure channel and not do anything with it. Or monitor a value that never changes.

So setting a ReadDeadline applies to all messages and means that the user expects the server to always send a message at least every so often. Otherwise, we get an error. I think the only sensible thing to do is to close the connection.

magiconair avatar Jun 14 '23 05:06 magiconair

context deadlines are specific to a request and shouldn't necessarily close the connection or am I off-base here?

magiconair avatar Jun 14 '23 06:06 magiconair

Context terms don't close the connection, it's local logic If we send/write something from conn and the deadline has passed, we will just get an error and have to determine what we want We can increase the deadline value for the second send/write operation and close the connection after the third attempt, for example. https://pkg.go.dev/net#Conn

Yes, ReadDeadline applies to all messages for the conn instance, and after reading it is more correct to set the value to SetReadDeadline(time.Time{}). This correct for WriteDeadline too

cckolya avatar Jul 11 '23 13:07 cckolya