Merge pull request #997 from qiangdavidliu/master

Add ability to fail fast in DiscoveryClient construction if an initial registration attempt fails
Netflix · Oct 9, 2017 · d4fcefc · d4fcefc
2 parents d2dbee8 + bdcf59c
commit d4fcefc
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 6 deletions.
diff --git a/eureka-client-archaius2/src/main/java/com/netflix/discovery/EurekaArchaius2ClientConfig.java b/eureka-client-archaius2/src/main/java/com/netflix/discovery/EurekaArchaius2ClientConfig.java
@@ -209,6 +209,10 @@ public boolean shouldOnDemandUpdateStatusChange() {
         return prefixedConfig.getBoolean(SHOULD_ONDEMAND_UPDATE_STATUS_KEY, true);
     }
 
+    public boolean shouldEnforceRegistrationAtInit() {
+        return prefixedConfig.getBoolean(SHOULD_ENFORCE_REGISTRATION_AT_INIT, false);
+    }
+
     @Override
     public String getEncoderName() {
         return prefixedConfig.getString(CLIENT_ENCODER_NAME_KEY, null);

diff --git a/eureka-client/src/main/java/com/netflix/discovery/DefaultEurekaClientConfig.java b/eureka-client/src/main/java/com/netflix/discovery/DefaultEurekaClientConfig.java
@@ -487,6 +487,12 @@ public boolean shouldOnDemandUpdateStatusChange() {
                 namespace + SHOULD_ONDEMAND_UPDATE_STATUS_KEY, true).get();
     }
 
+    @Override
+    public boolean shouldEnforceRegistrationAtInit() {
+        return configInstance.getBooleanProperty(
+                namespace + SHOULD_ENFORCE_REGISTRATION_AT_INIT, false).get();
+    }
+
     @Override
     public String getEncoderName() {
         return configInstance.getStringProperty(

diff --git a/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java b/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java
@@ -417,6 +417,19 @@ public synchronized BackupRegistry get() {
         if (this.preRegistrationHandler != null) {
             this.preRegistrationHandler.beforeRegistration();
         }
+
+        if (clientConfig.shouldRegisterWithEureka() && clientConfig.shouldEnforceRegistrationAtInit()) {
+            try {
+                if (!register() ) {
+                    throw new IllegalStateException("Registration error at startup. Invalid server response.");
+                }
+            } catch (Throwable th) {
+                logger.error("Registration error at startup.", th.getMessage());
+                throw new IllegalStateException(th);
+            }
+        }
+
+        // finally, init the schedule tasks (e.g. cluster resolvers, heartbeat, instanceInfo replicator, fetch
         initScheduledTasks();
 
         try {

diff --git a/eureka-client/src/main/java/com/netflix/discovery/EurekaClientConfig.java b/eureka-client/src/main/java/com/netflix/discovery/EurekaClientConfig.java
@@ -517,6 +517,16 @@ public interface EurekaClientConfig {
      */
     boolean shouldOnDemandUpdateStatusChange();
 
+    /**
+     * If set to true, the {@link EurekaClient} initialization should throw an exception at constructor time
+     * if an initial registration to the remote servers is unsuccessful.
+     *
+     * Note that if {@link #shouldRegisterWithEureka()} is set to false, then this config is a no-op
+     *
+     * @return true or false for whether the client initialization should enforce an initial registration
+     */
+    boolean shouldEnforceRegistrationAtInit();
+
     /**
      * This is a transient config and once the latest codecs are stable, can be removed (as there will only be one)
      *

diff --git a/eureka-client/src/main/java/com/netflix/discovery/PropertyBasedClientConfigConstants.java b/eureka-client/src/main/java/com/netflix/discovery/PropertyBasedClientConfigConstants.java
@@ -24,6 +24,7 @@ final class PropertyBasedClientConfigConstants {
 
     static final String SHOULD_UNREGISTER_ON_SHUTDOWN_KEY = "shouldUnregisterOnShutdown";
     static final String SHOULD_ONDEMAND_UPDATE_STATUS_KEY = "shouldOnDemandUpdateStatusChange";
+    static final String SHOULD_ENFORCE_REGISTRATION_AT_INIT = "shouldEnforceRegistrationAtInit";
     static final String SHOULD_DISABLE_DELTA_KEY = "disableDelta";
     static final String SHOULD_FETCH_REMOTE_REGION_KEY = "fetchRemoteRegionsRegistry";
     static final String SHOULD_FILTER_ONLY_UP_INSTANCES_KEY = "shouldFilterOnlyUpInstances";

diff --git a/eureka-client/src/main/java/com/netflix/discovery/TimedSupervisorTask.java b/eureka-client/src/main/java/com/netflix/discovery/TimedSupervisorTask.java
@@ -65,7 +65,7 @@ public void run() {
             delay.set(timeoutMillis);
             threadPoolLevelGauge.set((long) executor.getActiveCount());
         } catch (TimeoutException e) {
-            logger.error("task supervisor timed out", e);
+            logger.warn("task supervisor timed out", e);
             timeoutCounter.increment();
 
             long currentDelay = delay.get();
@@ -76,15 +76,15 @@ public void run() {
             if (executor.isShutdown() || scheduler.isShutdown()) {
                 logger.warn("task supervisor shutting down, reject the task", e);
             } else {
-                logger.error("task supervisor rejected the task", e);
+                logger.warn("task supervisor rejected the task", e);
             }
 
             rejectedCounter.increment();
         } catch (Throwable e) {
             if (executor.isShutdown() || scheduler.isShutdown()) {
                 logger.warn("task supervisor shutting down, can't accept the task");
             } else {
-                logger.error("task supervisor threw an exception", e);
+                logger.warn("task supervisor threw an exception", e);
             }
 
             throwableCounter.increment();

diff --git a/eureka-client/src/test/java/com/netflix/discovery/EurekaClientLifecycleTest.java b/eureka-client/src/test/java/com/netflix/discovery/EurekaClientLifecycleTest.java
@@ -7,6 +7,7 @@
 
 import com.google.inject.AbstractModule;
 import com.google.inject.Injector;
+import com.google.inject.ProvisionException;
 import com.google.inject.Scopes;
 import com.netflix.appinfo.EurekaInstanceConfig;
 import com.netflix.appinfo.InstanceInfo;
@@ -118,7 +119,7 @@ public void testBackupRegistryInjection() throws Exception {
                             @Override
                             protected void configure() {
                                 bind(EurekaInstanceConfig.class).to(LocalEurekaInstanceConfig.class);
-                                bind(EurekaClientConfig.class).to(BadServerEurekaClientConfig.class);
+                                bind(EurekaClientConfig.class).to(BadServerEurekaClientConfig1.class);
                                 bind(BackupRegistry.class).toInstance(backupRegistry);
                                 bind(AbstractDiscoveryClientOptionalArgs.class).to(Jersey1DiscoveryClientOptionalArgs.class).in(Scopes.SINGLETON);
                             }
@@ -133,6 +134,31 @@ protected void configure() {
         assertThat(countInstances(client.getApplications()), is(equalTo(1)));
     }
 
+    @Test(expected = ProvisionException.class)
+    public void testEnforcingRegistrationOnInitFastFail() {
+        Injector injector = LifecycleInjector.builder()
+                .withModules(
+                        new AbstractModule() {
+                            @Override
+                            protected void configure() {
+                                bind(EurekaInstanceConfig.class).to(LocalEurekaInstanceConfig.class);
+                                bind(EurekaClientConfig.class).to(BadServerEurekaClientConfig2.class);
+                                bind(AbstractDiscoveryClientOptionalArgs.class).to(Jersey1DiscoveryClientOptionalArgs.class).in(Scopes.SINGLETON);
+                            }
+                        }
+                )
+                .build().createInjector();
+        LifecycleManager lifecycleManager = injector.getInstance(LifecycleManager.class);
+        try {
+            lifecycleManager.start();
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        // this will throw a Guice ProvisionException for the constructor failure
+        EurekaClient client = injector.getInstance(EurekaClient.class);
+    }
+
     private static class LocalEurekaInstanceConfig extends PropertiesInstanceConfig {
 
         @Override
@@ -173,15 +199,32 @@ public int getRegistryFetchIntervalSeconds() {
         }
     }
 
-    private static class BadServerEurekaClientConfig extends LocalEurekaClientConfig {
+    private static class BadServerEurekaClientConfig1 extends LocalEurekaClientConfig {
         @Override
         public List<String> getEurekaServerServiceUrls(String myZone) {
-            return singletonList("http://localhost:0/v2/"); // Fail fast on bad port number
+            return singletonList("http://localhost:1/v2/"); // Fail fast on bad port number
         }
 
         @Override
         public boolean shouldRegisterWithEureka() {
             return false;
         }
     }
+
+    private static class BadServerEurekaClientConfig2 extends LocalEurekaClientConfig {
+        @Override
+        public List<String> getEurekaServerServiceUrls(String myZone) {
+            return singletonList("http://localhost:1/v2/"); // Fail fast on bad port number
+        }
+
+        @Override
+        public boolean shouldFetchRegistry() {
+            return false;
+        }
+
+        @Override
+        public boolean shouldEnforceRegistrationAtInit() {
+            return true;
+        }
+    }
 }