Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cthulhu-rider opened this issue Nov 25, 2024 · 2 comments
Open
Labels
client Issue related to the client config Configuration format update or breaking change discussion Open discussion of some problem I3 Minimal impact S3 Minimally significant U4 Nothing urgent

Comments

@cthulhu-rider
Copy link
Contributor

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

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 #636

@cthulhu-rider cthulhu-rider added client Issue related to the client discussion Open discussion of some problem config Configuration format update or breaking change labels Nov 25, 2024
@roman-khimov roman-khimov added U4 Nothing urgent S3 Minimally significant I3 Minimal impact labels Nov 25, 2024
@roman-khimov
Copy link
Member

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.

@cthulhu-rider
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Issue related to the client config Configuration format update or breaking change discussion Open discussion of some problem I3 Minimal impact S3 Minimally significant U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

2 participants