Spring-cloud-netflix: Customize the load balancing rule with the @Bean annotation, but get the wrong LoadBalancer.

[hi-sb]

@Bean
public IRule getIRule(){ return xxx;}

This way uses a custom load balancing policy. After a request to call two consecutive service, again to re-request, in my realization of the IRule choose (Object key) method to get the wrong list of services.

```
BaseLoadBalancer.java

public Server chooseServer(Object key) {
if (counter == null) {
counter = createCounter();
}
counter.increment();
if (rule == null) {
return null;
} else {
try {
//Do you need to add this?
rule.setLoadBalancer( this );
return rule.choose(key);
} catch (Exception e) {
logger.warn("LoadBalancer [{}]: Error choosing server for key {}", name, key, e);
return null;
}
}
}
```

[ryanjbaxter]

Please format code properly to make it easier to read.

I am not really sure what the problem is, are you saying the wrong LoadBalancer is set in the IRule?

[hi-sb]

Yes, LoadBalancer gets the list of current service server, but gets the server list of another service.After viewing the source code, I think need to add rule.setLoadBalancer (this);

[ryanjbaxter]

If you would like to submit a PR with a fix that would be welcomed. Just make sure you include tests that demonstrate the problem/fix.

[spencergibb]

@hi-sb that means your IRule is getting picked up outside of @RibbonClient. Please show the whole class where the IRule is defined and where the @RibbonClient is defined. It's likely getting picked up by component scanning.

[hi-sb]

I try to use this method to specify an IRule

    @Bean
    public IRule getIRule() {
        return  new com.netflix.loadbalancer.RoundRobinRule();
    }

Then try alternately visiting test1, and test2.

  @Autowired
    RestTemplate restTemplate;

    @Override
    @RequestMapping("test1")
    public ResponseEntity<String> test1() {
        return  restTemplate.getForEntity("http://services-node/service-node/application-node", String.class );
    }

    @RequestMapping("test2")
    public ResponseEntity<String> test2(){
        return  restTemplate.getForEntity("http://authentication-service/user/xxxxx",String.class );
    }

The result throws an exception: org.springframework.web.client.HttpClientErrorException: 404 nulll.

RoundRobinRule.java
public Server choose(ILoadBalancer lb, Object key) {
        if (lb == null) {
            log.warn("no load balancer");
            return null;
        }

        Server server = null;
        int count = 0;
        while (server == null && count++ < 10) {
            List<Server> reachableServers = lb.getReachableServers();
            List<Server> allServers = lb.getAllServers();
            int upCount = reachableServers.size();
            int serverCount = allServers.size();

            if ((upCount == 0) || (serverCount == 0)) {
                log.warn("No up servers available from load balancer: " + lb);
                return null;
            }

            int nextServerIndex = incrementAndGetModulo(serverCount);
            server = allServers.get(nextServerIndex);

            if (server == null) {
                /* Transient. */
                Thread.yield();
                continue;
            }

            if (server.isAlive() && (server.isReadyToServe())) {
                return (server);
            }

            // Next.
            server = null;
        }

        if (count >= 10) {
            log.warn("No available alive servers after 10 tries from load balancer: "
                    + lb);
        }
        return server;
    }

When you access the services-node service, you get the server list for the authentication-service service.

I think we should modify BaseLoadBalancer.java.

public Server chooseServer(Object key) {
       if (counter == null) {
           counter = createCounter();
       }
       counter.increment();
       if (rule == null) {
           return null;
       } else {
           try {
               //Do you need to add this?
               rule.setLoadBalancer( this );
               return rule.choose(key);
           } catch (Exception e) {
               logger.warn("LoadBalancer [{}]:  Error choosing server for key {}", name, key, e);
               return null;
           }
       }
   }

[ryanjbaxter]

Have you looked at ClientConfigEnabledRoundRobinRule? It looks like it sets the LoadBalancer.
https://github.com/Netflix/ribbon/blob/5e6066dd63253014599b4fd89713a7b51f19d165/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/ClientConfigEnabledRoundRobinRule.java#L41

[spencergibb]

Please show the whole class where the IRule is defined and where the @RibbonClient is defined. It's likely getting picked up by component scanning.

[hi-sb]

Sorry, I gave you a bug fix.After reading the code carefully, I found the reason for the error.

    @Bean
    public IRule getIRule() {
        return  new com.netflix.loadbalancer.RoundRobinRule();
    }

This actually creates a single instance of IRule.Then, the two service ILoadBalancer stores the same IRule address reference.

When the second LoadBalancer calls setLoadBalancer (ILoadBalancer lb) on its own Irule, it actually covers the other.

   @Bean
   @Scope("prototype")
    public IRule getIRule() {
        return  new com.netflix.loadbalancer.RoundRobinRule();
    }

Change to multiple instances to solve problems.I don't know if it's a BUG.

[nmnellis]

We have a very similar issue but we only notice it when the backend services go down. Ribbon will select a different serverList that we have set up for other routes and stick to them. We use a pick first custom rule shown below. I am going to attempt your prototype fix now.

Custom Rule Definition

    @Bean
    @Scope("prototype")
    public IRule ribbonRule() {
        return new PickFirstLoadBalancerRule();
    }

Pick First Load Balancer Rule

/**
 * Chooses the first healthy instance in the list
 */
public class PickFirstLoadBalancerRule extends AbstractLoadBalancerRule {
    @Override
    public void initWithNiwsConfig(IClientConfig clientConfig) {

    }

    @Override
    public Server choose(Object key) {

        List<Server> upList = getLoadBalancer().getReachableServers();
        //returns the first lb in the list every time
        if (upList != null && upList.size() > 0) {
            return upList.get(0);
        }
        return null;
    }
}

[spencergibb]

@hi-sb and @nmnellis using prototype scope isn't the proper way to do things. If you can show me the full class where the IRule is defined and where @RibbonClient is defined (or a sample project) I can show you what is wrong.

[nmnellis]

@spencergibb we are using property files to load the serverlist and we noticed that if app2 backend would go down for an extended period of time it would use the serverlist from app1. my previous comment is the full IRule.

zuul:
  routes:
    app1:
      path: /v1/app1
      proxy: /v1/backend1
    app2:
      path: /v1/app2
      proxy: /v1/backend2
app1:
  ribbon:
    listOfServers: http://localhost:8080
app2:
  ribbon:
    listOfServers: http://localhost:8081

[spencergibb]

I'm looking for the full class where the @Bean of your IRule is defined and the full class of @RibbonClient

[nmnellis]

Sorry, we do not use @RibbonClient. Here is our application configuration. The CustomRibbonRoutingFilter extends RibbonRoutingFilter and the only change made was to allow inspection of the request to do a url rewrite to the backend using the proxy property above.

@Configuration
public class AppConfig {
    /**
     * Custom Ribbon Routing Filter to allow for modification of the request as it passes
     *
     * @param ribbonCommandFactory
     * @return
     */
    @Bean
    public ZuulFilter backendPreFilter(RibbonCommandFactory<?> ribbonCommandFactory) {
        return new CustomRibbonRoutingFilter(ribbonCommandFactory);
    }

    /**
     * This filter says use the first url until it is un
     *
     * @return
     */
    @Bean
    public IRule ribbonRule() {
        return new PickFirstLoadBalancerRule();
    }

    /**
     * Filter for throttling requests
     *
     * @return
     */
    @Bean
    public ZuulFilter throttlingFilter() {
        return new ThrottlingFilter();
    }

    @Bean
    public ZuulFilter postErrorFilter() {
        return new PostErrorFilter();
    }
}

App.java

@SpringBootApplication
@EnableScheduling
@EnableDiscoveryClient
@EnableZuulProxy
public class App {

    public static void main(String[] args) {
        run(args);
    }

    public static ConfigurableApplicationContext run(String[] args) {
    }

}

Pom

        <spring.cloud.netflix.version>1.2.2.RELEASE</spring.cloud.netflix.version>
...
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-zuul</artifactId>
            <version>${spring.cloud.netflix.version}</version>
        </dependency>

[spencergibb]

Using @RibbonClient is the documented way to customize the ribbon rules. Without using it, you will see the problems that you encounter.

[nmnellis]

@spencergibb my application has 6 ribbon clients currently and it appears that @RibbonClient is not a repeatable annotation. can i use properties instead as stated in the documentation?

Complains that annotation is not repeatable

@Configuration
@RibbonClient(name = "app1", configuration = RibbonConfig.class)
@RibbonClient(name = "app2", configuration = RibbonConfig.class)
public class AppConfig{

}

Can i use this instead?

app1:
  ribbon:
    NFLoadBalancerRuleClassName: com.my.lb.PickFirstLoadBalancerRule
app2:
  ribbon:
    NFLoadBalancerRuleClassName: com.my.lb.PickFirstLoadBalancerRule

[spencergibb]

Yes, @RibbonClients allows for repeated annotations. It also allows for defaults for all ribbon clients

[spencergibb]

Closing this due to inactivity. Please re-open if there's more to discuss.