Stream timeout seems too artificial and goes beyond the gRPC scope #240

Open
opened 2025-12-28 18:07:32 +00:00 by sami · 3 comments
Owner

Originally created by @cthulhu-rider on GitHub (Nov 25, 2024).

https://pkg.go.dev/github.com/nspcc-dev/neofs-sdk-go/client#PrmDial.SetStreamTimeout setting timeout for the single stream message. The implementation is here

i see following cons about the existence of such a setting from the user pov:

  1. gRPC lib works with the full stream timeout only, single message timeout requires manual implementations
  2. may conflict with the context timeout of the full operation
  3. delaying one message can fail the entire stream in cases where messages could be delivered at the context "end"
  4. the parameter is static for the Client and does not depend on the current state. Moreover, current state of the system/network load is not transparent to the user

Describe the solution you'd like

purge this parameter and rely on user context only. This approach is adopted in the gRPC, and it is a good abstraction from the details of data transmission in the form of a stream: the user understands the complete operation only.

Describe alternatives you've considered

still drop the parameter, and also we can try to invent restrictions under the hood based on a user deadline. But for this we need to first build a working model, which should be based on the transport properties reported by the server. For now, this seems to artificial, and single deadline around transport abstraction seems best to me

Additional context

i remembe the parameter during refactoring https://github.com/nspcc-dev/neofs-sdk-go/pull/636

Originally created by @cthulhu-rider on GitHub (Nov 25, 2024). ## Is your feature request related to a problem? Please describe. https://pkg.go.dev/github.com/nspcc-dev/neofs-sdk-go/client#PrmDial.SetStreamTimeout setting timeout for the single stream message. The implementation is [here](https://github.com/nspcc-dev/neofs-api-go/commit/aa53fb7131cd96f3ab70729827d18d63f92917d6) i see following cons about the existence of such a setting from the user pov: 1. gRPC lib works with the full stream timeout only, single message timeout requires manual implementations 2. may conflict with the context timeout of the full operation 3. delaying one message can fail the entire stream in cases where messages could be delivered at the context "end" 4. the parameter is static for the `Client` and does not depend on the current state. Moreover, current state of the system/network load is not transparent to the user ## Describe the solution you'd like purge this parameter and rely on user context only. This approach is adopted in the gRPC, and it is a good abstraction from the details of data transmission in the form of a stream: the user understands the complete operation only. ## Describe alternatives you've considered still drop the parameter, and also we can try to invent restrictions under the hood based on a user deadline. But for this we need to first build a working model, which should be based on the transport properties reported by the server. For now, this seems to artificial, and single deadline around transport abstraction seems best to me ## Additional context i remembe the parameter during refactoring https://github.com/nspcc-dev/neofs-sdk-go/pull/636
Author
Owner

@roman-khimov commented on GitHub (Nov 25, 2024):

The main problem here is 1TB object GET. Per-request deadlines are perfect when objects are small. Once you have 1TB of data it becomes a bit different. What's the good timeout for this operation? Per-message timeouts allow to catch problems earlier in this case, but they're not perfect either.

@roman-khimov commented on GitHub (Nov 25, 2024): The main problem here is 1TB object GET. Per-request deadlines are perfect when objects are small. Once you have 1TB of data it becomes a bit different. What's the good timeout for this operation? Per-message timeouts allow to catch problems earlier in this case, but they're not perfect either.
Author
Owner

@cthulhu-rider commented on GitHub (Nov 25, 2024):

What's the good timeout for this operation?

there is no universal answer cuz GET user does not know the data size in general. If he is constrained by time, he will set a timeout for the entire operation. If not, there will be no deadline at all. Moreover, given the provided stream interfaces, the user can wrap his code with artificial deadlines himself. However, in almost all cases he will not want to do this

@cthulhu-rider commented on GitHub (Nov 25, 2024): > What's the good timeout for this operation? there is no universal answer cuz GET user does not know the data size in general. If he is constrained by time, he will set a timeout for the entire operation. If not, there will be no deadline at all. Moreover, given the provided stream interfaces, the user can wrap his code with artificial deadlines himself. However, in almost all cases he will not want to do this
Author
Owner

@cthulhu-rider commented on GitHub (May 5, 2025):

lookin at https://pkg.go.dev/google.golang.org/grpc#ClientStream

// SendMsg does not wait until the message is received by the server. An
// untimely stream closure may result in lost messages.

i noticed one more reason why such timeout makes no sense for client-side streams in particular (Object PUT only for now). This is very visible in practice. I measured time spent for nspcc-dev/neofs-sdk-go@1af995ff59/client/object_put.go (L34) calls. All of them are completed in nanos, cuz they aint written to the wire insta. And only nspcc-dev/neofs-sdk-go@1af995ff59/client/object_put.go (L36) takes a significant amount of time, almost equal to the whole duration

so, for client streams, the option is not applicable in normal mode


server-side streams behave similarly, it's just that it's not requests that are cached but responses

@cthulhu-rider commented on GitHub (May 5, 2025): lookin at https://pkg.go.dev/google.golang.org/grpc#ClientStream > // SendMsg does not wait until the message is received by the server. An // untimely stream closure may result in lost messages. i noticed one more reason why such timeout makes no sense for client-side streams in particular (Object PUT only for now). This is very visible in practice. I measured time spent for https://github.com/nspcc-dev/neofs-sdk-go/blob/1af995ff59815aa59aba913d8481523b0e71de64/client/object_put.go#L34 calls. All of them are completed in nanos, cuz they aint written to the wire insta. And only https://github.com/nspcc-dev/neofs-sdk-go/blob/1af995ff59815aa59aba913d8481523b0e71de64/client/object_put.go#L36 takes a significant amount of time, almost equal to the whole duration so, for client streams, the option is not applicable in normal mode --- server-side streams behave similarly, it's just that it's not requests that are cached but responses
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
nspcc-dev/neofs-sdk-go#240
No description provided.