SimpleHostRoutingFilter doesn't honor Zuul timeouts anymore

[asarkar]

In Camden.SR5, SimpleHostRoutingFilter used to set the socket and connection timeouts based on the properties zuul.host.socket-timeout-millis and zuul.host.connect-timeout-millis, respectively. In Dalston.RELEASE, these properties have no effect, and the default values in the code are used.

[ryanjbaxter]

Can you point to where the change occurred? Quickly looking at the code it looks like we are still using those properties.

[asarkar]

@ryanjbaxter I didn't try and find out the code but simply putting a breakpoint in the class with before and after upgrade showed that the default values are picked up afterwards. I suppose it won't be terribly difficult to create a project to demonstrate this issue. Do you've any integration tests that test this feature?

[spencergibb]

There was no test. 7425f974cb2e45ea86effe15779d8ad3e5b366a1 added one. Can you describe your situation more than "these properties have no effect"? What is your configuration? Anything else that changed between Camden.SR5 and Dalston.RELEASE?

[asarkar]

@spencergibb

Anything else that changed between Camden.SR5 and Dalston.RELEASE?

If you mean in Zuul, that affected us other than this, https://github.com/spring-cloud/spring-cloud-netflix/issues/1874#issuecomment-301621558

I've attached a Gradle project that reproduces the timeout issue. There are 2 tests (one for https://github.com/spring-cloud/spring-cloud-netflix/issues/1988), all you've to do is run the ZuulTimeoutTest.

demo.zip

[asarkar]

Ding!

[spencergibb]

Working on a fix. It's a combination of early initialization and archaius. Moving to @ConfigurationProperties.

[asarkar]

Thanks @spencergibb. Like I suspected, it's a bug, so probably should fix the "waiting for feedback" tag.

[spencergibb]

Closed via e08a6e8a486ada974da9754a4264650e17eefea6

[asarkar]

@spencergibb This defect is because of the same root cause. Can you comment whether https://github.com/spring-cloud/spring-cloud-netflix/commit/e08a6e8a486ada974da9754a4264650e17eefea6 will fix this too?

[spencergibb]

I clearly commented above that e08a6e8 closed this issue and the commit comment says "fixes gh-1950". I'm confused about what you are asking.

[asarkar]

I'm asking about a different issue (forgot to put the link initially, sorry). I've now updated my comment.

[spencergibb]

I'd say no.

[asarkar]

@spencergibb What direction would you recommend taking? The reporter of the other bug is importing ArchaiusAutoConfiguration and then injecting it to force initialization. That seems like a bad hack to me. If https://github.com/spring-cloud/spring-cloud-netflix/commit/e08a6e8a486ada974da9754a4264650e17eefea6 didn't fix https://github.com/Netflix/Hystrix/issues/1428, should I give up on Javanica @HystrixCommand and create a HystrixCommand manually using a @ConfigurationProperties (basically putting Archaius out of the picture)? That seems like a lot of work.

[asarkar]

By putting a breakpoint in com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect.methodsAnnotatedWithHystrixCommand(), I can see the Hystrix command being executed with the expected threadpool values. So in my case, the ArchaiusAutoConfiguration being initialized later is not an issue because the AOP is executed for every request.

[asarkar]

@spencergibb Will this fix be available in Dalston SR2? I think along with the component version, a label for the platform release will also be very helpful in tickets.

[spencergibb]

Yes.