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

Feign Retryer is not configurable and overlaps with the ribbon retry. #467

Closed
LutzStrobel opened this Issue Jul 29, 2015 · 7 comments

Comments

Projects
None yet
6 participants
@LutzStrobel

LutzStrobel commented Jul 29, 2015

I have a consumer service calling an provider service which is running with 2 instances.
The provider interface

  @FeignClient("example")
  public interface ExampleService {
     @RequestMapping(
        method = {RequestMethod.POST},
        value = {"/greeting/{nameToGreat}"}
      )
     String greeting(@PathVariable(value = "nameToGreat") String nameToGreat);

The ribbon configuration within the consumer service is as follows:

example.ribbon.OkToRetryOnAllOperations=false
example.ribbon.ReadTimeout=3999
example.ribbon.MaxAutoRetries=0
example.ribbon.MaxAutoRetriesNextServer=3

If there is a SocketTimeoutException during a call to the example service, I would not expect that there a retry to the next instance occurs, because POST requests are not idempotent.
But the real behavior is that there retries to the next instance occurs which causes creation of duplicate accounts aso.

In a debugging session I could see that a correct RequestSpecificRetryHandler with the ribbon config values from above will be created and as far I could see the method isRetriableException(...) of that handler returns false.

What can I do to find a solution?
Thanx
Lutz

@LutzStrobel

This comment has been minimized.

Show comment
Hide comment
@LutzStrobel

LutzStrobel Jul 29, 2015

Now I found that the FeignClient uses the default retryer which retries the failed requests on its own.
I think this is not desired, because it overrules the configuration for the ribbon loadbalancer.
Maybe this is not a feature, is it?
Lutz

LutzStrobel commented Jul 29, 2015

Now I found that the FeignClient uses the default retryer which retries the failed requests on its own.
I think this is not desired, because it overrules the configuration for the ribbon loadbalancer.
Maybe this is not a feature, is it?
Lutz

@spencergibb spencergibb added Backlog and removed Backlog labels Nov 17, 2015

@spencergibb spencergibb changed the title from LoadBalancerCommand retries post requests even if configured: OkToRetryOnAllOperations=false to Feign Retryer is not configurable and overlaps with the ribbon retry. Mar 3, 2016

@spencergibb

This comment has been minimized.

Show comment
Hide comment
@spencergibb

spencergibb Mar 3, 2016

Member

Perhaps the default should be an empty Retryer.

As a workaround, you can create a Retryer bean in a Feign Client Configuration.

Member

spencergibb commented Mar 3, 2016

Perhaps the default should be an empty Retryer.

As a workaround, you can create a Retryer bean in a Feign Client Configuration.

@padilo

This comment has been minimized.

Show comment
Hide comment
@padilo

padilo Apr 25, 2016

Workaround works like a charm:

  @Bean
  public Retryer retryer() {
    return new Retryer() {

      @Override
      public void continueOrPropagate(RetryableException e) {
        throw e;
      }

      @Override
      public Retryer clone() {
        return this;
      }
    };
  }

padilo commented Apr 25, 2016

Workaround works like a charm:

  @Bean
  public Retryer retryer() {
    return new Retryer() {

      @Override
      public void continueOrPropagate(RetryableException e) {
        throw e;
      }

      @Override
      public Retryer clone() {
        return this;
      }
    };
  }
@srowatt

This comment has been minimized.

Show comment
Hide comment
@srowatt

srowatt Apr 6, 2017

There's a NEVER_RETRY implementation so you can do

@Bean public Retryer retryer() { return Retryer.NEVER_RETRY; }

srowatt commented Apr 6, 2017

There's a NEVER_RETRY implementation so you can do

@Bean public Retryer retryer() { return Retryer.NEVER_RETRY; }

@ryanjbaxter

This comment has been minimized.

Show comment
Hide comment
@ryanjbaxter

ryanjbaxter Apr 6, 2017

Contributor

We can close this issue because all this retry logic was simplified in the latest Camden SRs and and Dalston snapshots and there is only one level of retry now.

Contributor

ryanjbaxter commented Apr 6, 2017

We can close this issue because all this retry logic was simplified in the latest Camden SRs and and Dalston snapshots and there is only one level of retry now.

@ryanjbaxter ryanjbaxter closed this Apr 6, 2017

@Theknickerbocker

This comment has been minimized.

Show comment
Hide comment
@Theknickerbocker

Theknickerbocker Jul 18, 2017

Can you explain what you mean by 'there is only one level of retry now'?

Theknickerbocker commented Jul 18, 2017

Can you explain what you mean by 'there is only one level of retry now'?

@spencergibb

This comment has been minimized.

Show comment
Hide comment
@spencergibb

spencergibb Jul 18, 2017

Member

We've eliminated the feign retry with our ribbon retry

Member

spencergibb commented Jul 18, 2017

We've eliminated the feign retry with our ribbon retry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment