hystrix-go icon indicating copy to clipboard operation
hystrix-go copied to clipboard

Provide context for runFunc which is cancelled during timeouts

Open cyrusaf opened this issue 7 years ago • 2 comments

Currently, when running hystrix.Do or hystrix.Go, there is no built in context cancellation during a timeout. For example, when making a dynamo query:

err := hystrix.Do(HystrixCommand, func() error {
    results, err := this.ddb.QueryWithContext(ctx, query)
    return err
}, nil)

If hystrix times out here, the dynamo query will not be cancelled (since the context is not cancelled) and may stay open forever. I had a case recently, where I reached my dynamo read capacity and this query stayed open for > 80 seconds.

It is possible to add my own context cancellation:

ctx, cancel := context.WithCancel(ctx)
err := hystrix.Do(HystrixCommand, func() error {
    results, err := this.ddb.QueryWithContext(ctx, query)
    return err
}, func(err error) error {
    cancel()
    return nil
})

But I think it should be built into the library:

err := hystrix.Do(ctx, HystrixCommand, func(ctx context.Context) error {
    results, err := this.ddb.QueryWithContext(ctx, query)
    return err
}, nil)

Where the passed in context to hystrix.Do is wrapped with context.WithCancel(ctx), passed into the runFunc, and then the cancel func is called when there is an error/timeout.

cyrusaf avatar Feb 20 '18 19:02 cyrusaf

this feature is introduced in https://github.com/afex/hystrix-go/pull/79

please try out that branch and let me know if it fits your needs

afex avatar Feb 20 '18 20:02 afex

Just took a look at https://github.com/afex/hystrix-go/pull/79 (which I see has been merged). Doesn't appear that the context is canceled when the circuit times out 🤔

jordanpotter avatar May 16 '18 21:05 jordanpotter