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

Favor timeout property that is not thread-specific. (execution.timeoutInMilliseconds) #664

Merged

Conversation

mattrjacobs
Copy link
Contributor

Also deprecate usages of thread-specific setting (like execution.thread.isolation.timeoutInMilliseconds).

This addresses #300

…tInMilliseconds)

* Deprecate usages of thread-specific setting (like execution.thread.isolation.timeoutInMilliseconds)
@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #30 SUCCESS
This pull request looks good

mattrjacobs added a commit that referenced this pull request Feb 10, 2015
Favor timeout property that is not thread-specific. (execution.timeoutInMilliseconds)
@mattrjacobs mattrjacobs merged commit 7cfb1f9 into Netflix:master Feb 10, 2015
@mattrjacobs mattrjacobs deleted the timeout-not-specific-to-threads branch February 10, 2015 06:30
@holy12345
Copy link

holy12345 commented Jan 9, 2018

Hi @mattrjacobs
I have a question want your confirmation.
In the PR you submitted I saw an addition of an execution.timeoutInMilliseconds property, which represents the timeout period (regardless of whether the isolation strategy is semaphore or thread).
I understand that this change can be seen in 1.4RC or later.

But my current version is 1.5.12. The execution.timeoutInMilliseconds property does not exist.

I saw the following code in HystrixCommandProperties

 //this property name is now misleading. //TODO figure out a good way to deprecate this property name
this.executionTimeoutInMilliseconds = getProperty(propertyPrefix, key, "execution.isolation.thread.timeoutInMilliseconds", builder.getExecutionIsolationThreadTimeoutInMilliseconds(), default_executionTimeoutInMilliseconds);

By javadoc and my local validation I got the following conclusion:

  • In the 1.5.12 version we did not execute.timeoutInMilliseconds this parameter.

  • Currently we are still using execution.isolation.thread.timeoutInMilliseconds to represent the timeout (regardless of whether the isolation strategy is semaphore or thread, as javadoc said, the name of this parameter is confusing)

Am i right? Looking forward to your reply. Thanks again, best wishes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants