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.

Sign up for GitHub

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

Jump to bottom

Use context during client dial #343

Closed
alexvanin opened this issue Sep 23, 2022 · 0 comments · Fixed by #346
Closed

Use context during client dial #343

alexvanin opened this issue Sep 23, 2022 · 0 comments · Fixed by #346
Assignees
@cthulhu-rider
Labels
bug Something isn't working client Issue related to the client enhancement Improving existing functionality U3 Regular
Milestone
v1.0.0-rc8

Comments

@alexvanin
Copy link
Contributor

Problem

Client's Dial() does not process application level context.

neofs-sdk-go/client/client.go

Line 73 in 8e3173e

func (c *Client) Dial(prm PrmDial) error {

neofs-sdk-go/client/client.go

Lines 167 to 177 in 8e3173e

type PrmDial struct {
endpoint string
tlsConfig *tls.Config
timeoutDialSet bool
timeoutDial time.Duration
streamTimeoutSet bool
streamTimeout time.Duration
}

If connection can't be established, application may freeze until timeout (20 seconds) and it will ignore closed application context. See (nspcc-dev/neofs-s3-gw#712)

Solution

Pass context into Dial function and use it during initial balance request as client.WithContext() option. It worked in PoC: (nspcc-dev/neofs-s3-gw#712 (comment))
@alexvanin alexvanin mentioned this issue Sep 23, 2022
Closed
@alexchetaev alexchetaev added U3 Regular 2022Q3 and removed U2 Seriously planned labels Sep 30, 2022
@cthulhu-rider cthulhu-rider self-assigned this Oct 3, 2022
@cthulhu-rider cthulhu-rider added enhancement Improving existing functionality client Issue related to the client and removed triage labels Oct 3, 2022
cthulhu-rider added a commit to cthulhu-rider/neofs-sdk-go that referenced this issue Oct 3, 2022
@cthulhu-rider
[nspcc-dev#343] client: Accept context parameter in Dial
45542f5 
In previous implementation of `Client.Dial` there was no ability to
specify parent context (e.g. global application context).

Add `PrmDial.SetContext` method which accepts optional base dial
context. Use the context to open client connection or fall back to using
`context.Background()`.

Upgraded version of `github.com/nspcc-dev/neofs-api-go/v2` module
also fixes the problem when dial timeout didn't work properly.
@cthulhu-rider cthulhu-rider mentioned this issue Oct 3, 2022
Merged
cthulhu-rider added a commit to cthulhu-rider/neofs-sdk-go that referenced this issue Oct 3, 2022
@cthulhu-rider
[nspcc-dev#343] pool: Provide dial context to clients
f33ed86 
After recent changes `client.Client` accepts dial context. There is a
need to forward the context passed into `Pool.Dial` to the underlying
`Client` instances.

Define type aliases of different client constructors: context-based and
non-context. Use context-based constructor in `Pool`. Pass `ctx`
parameter of `Pool.Dial` method to the client builder.
cthulhu-rider added a commit to cthulhu-rider/neofs-sdk-go that referenced this issue Oct 3, 2022
@cthulhu-rider
[nspcc-dev#343] client: Accept context parameter in Dial
298e2bf 
In previous implementation of `Client.Dial` there was no ability to
specify parent context (e.g. global application context).

Add `PrmDial.SetContext` method which accepts optional base dial
context. Use the context to open client connection or fall back to using
`context.Background()`.

Upgraded version of `github.com/nspcc-dev/neofs-api-go/v2` module
also fixes the problem when dial timeout didn't work properly.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider added a commit to cthulhu-rider/neofs-sdk-go that referenced this issue Oct 3, 2022
@cthulhu-rider
[nspcc-dev#343] pool: Provide dial context to clients
e4fa564 
After recent changes `client.Client` accepts dial context. There is a
need to forward the context passed into `Pool.Dial` to the underlying
`Client` instances.

Define type aliases of different client constructors: context-based and
non-context. Use context-based constructor in `Pool`. Pass `ctx`
parameter of `Pool.Dial` method to the client builder.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider added a commit to cthulhu-rider/neofs-sdk-go that referenced this issue Oct 5, 2022
@cthulhu-rider
[nspcc-dev#343] client: Accept context parameter in Dial
2289d61 
In previous implementation of `Client.Dial` there was no ability to
specify parent context (e.g. global application context).

Add `PrmDial.SetContext` method which accepts optional base dial
context. Use the context to open client connection or fall back to using
`context.Background()`.

Upgraded version of `github.com/nspcc-dev/neofs-api-go/v2` module
also fixes the problem when dial timeout didn't work properly.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider added a commit to cthulhu-rider/neofs-sdk-go that referenced this issue Oct 5, 2022
@cthulhu-rider
[nspcc-dev#343] pool: Provide dial context to clients
c3398dc 
After recent changes `client.Client` accepts dial context. There is a
need to forward the context passed into `Pool.Dial` to the underlying
`Client` instances.

Define type aliases of different client constructors: context-based and
non-context. Use context-based constructor in `Pool`. Pass `ctx`
parameter of `Pool.Dial` method to the client builder.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
@roman-khimov roman-khimov modified the milestones: v1.0.0, v1.0.0-rc8 Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cthulhu-rider cthulhu-rider

Labels
bug Something isn't working client Issue related to the client enhancement Improving existing functionality U3 Regular
Projects
None yet
Milestone
v1.0.0-rc8
Development

Successfully merging a pull request may close this issue.

client: Accept context parameter in Dial cthulhu-rider/neofs-sdk-go
5 participants
@alexvanin @fyrchik @roman-khimov @alexchetaev @cthulhu-rider