afex/hystrix-go

Provide context for runFunc which is cancelled during timeouts

cyrusaf opened this issue · 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.

afex commented

this feature is introduced in #79

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

Just took a look at #79 (which I see has been merged). Doesn't appear that the context is canceled when the circuit times out 🤔