HystrixTimeout vs HTTPTimeout

[v-byte-cpu]

What is the reason to have two timeouts: HystrixTimeout and HTTPTimeout for hystrix.Client ? It seems that two points are valid for current implementation:

1. timeout field in hystrix.Client is set but never used (maybe bug ?)
2. If any of the two events (HystrixTimeout or HTTPTimeout) occurs first then request will fail immediately. So if HTTPTimeout < HystrixTimeout, then HystrixTimeout is never used. And if HystrixTimeout < HTTPTimeout then hystrix will return error but goroutine with httpclient.Do will be still in blocking state waiting for response or http timeout (problem in hystrix code).

[milkwine]

It seems HTTPTimeout not realy affect hystrix.Client.client

heimdall/hystrix/hystrix_client.go

Lines 54 to 69 in e5b7967

	func NewClient(opts ...Option) *Client {
	client := Client{
	client: httpclient.NewClient(),
	timeout: defaultHTTPTimeout,
	hystrixTimeout: defaultHystrixTimeout,
	maxConcurrentRequests: defaultMaxConcurrentRequests,
	errorPercentThreshold: defaultErrorPercentThreshold,
	sleepWindow: defaultSleepWindow,
	requestVolumeThreshold: defaultRequestVolumeThreshold,
	retryCount: defaultHystrixRetryCount,
	retrier: heimdall.NewNoRetrier(),
	}
	for _, opt := range opts {
	opt(&client)
	}

heimdall/hystrix/options.go

Lines 21 to 26 in e5b7967

	// WithHTTPTimeout sets hystrix timeout
	func WithHTTPTimeout(timeout time.Duration) Option {
	return func(c *Client) {
	c.timeout = timeout
	}
	}

[Kaustavd]

I feel there should be 2 different timeouts as within hystrix method there might be other commands also getting executed like http connection setup (if connection is not reused) etc.

[pmitra43]

client.Timeout is not being passed on to hystrix setup. Ref: https://github.com/gojek/heimdall/blob/master/hystrix/hystrix_client.go#L80

It seems like a duplicate of the httpClient option. Ref: https://github.com/gojek/heimdall/blob/master/httpclient/client.go#L46

Is it safe to remove timeout config from hystrix and the WithHTTPTimeout option?