From fdc0a97553d3aecfa834e0c78f9dd1b94e3ff795 Mon Sep 17 00:00:00 2001 From: David Liu Date: Mon, 4 Jan 2016 18:19:36 -0800 Subject: [PATCH 1/2] add additional sanity check in shouldRefresh AmazonInfo --- .../netflix/appinfo/CloudInstanceConfig.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java b/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java index 82394b9518..840abc3259 100644 --- a/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java +++ b/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java @@ -163,7 +163,7 @@ public String[] getDefaultAddressResolutionOrder() { public synchronized void refreshAmazonInfo() { try { AmazonInfo newInfo = AmazonInfo.Builder.newBuilder().autoBuild(namespace); - if (!newInfo.equals(info)) { + if (shouldUpdate(newInfo, info)) { // the datacenter info has changed, re-sync it logger.warn("The AmazonInfo changed from : {} => {}", info, newInfo); this.info = newInfo; @@ -172,4 +172,19 @@ public synchronized void refreshAmazonInfo() { logger.error("Cannot refresh the Amazon Info ", t); } } + + private static boolean shouldUpdate(AmazonInfo newInfo, AmazonInfo oldInfo) { + if (newInfo.getMetadata().isEmpty()) { + logger.warn("Newly resolved AmazonInfo is empty, skipping an update cycle"); + return false; + } else if (!newInfo.equals(oldInfo)) { + int newKeySize = newInfo.getMetadata().size(); + int oldKeySize = oldInfo.getMetadata().size(); + if (newKeySize < oldKeySize) { + logger.warn("Newly resolved AmazonInfo contains less data than previous old:{} -> new:{}, skipping an update cycle", oldKeySize, newKeySize); + return false; + } + } + return true; + } } From 675e0843acb5ffaa55726f8caf1ed7c04f5ffde9 Mon Sep 17 00:00:00 2001 From: David Liu Date: Mon, 4 Jan 2016 19:25:57 -0800 Subject: [PATCH 2/2] logic fix + adding test case --- .../netflix/appinfo/CloudInstanceConfig.java | 8 ++--- .../appinfo/CloudInstanceConfigTest.java | 35 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java b/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java index 840abc3259..973fc20c62 100644 --- a/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java +++ b/eureka-client/src/main/java/com/netflix/appinfo/CloudInstanceConfig.java @@ -173,18 +173,18 @@ public synchronized void refreshAmazonInfo() { } } - private static boolean shouldUpdate(AmazonInfo newInfo, AmazonInfo oldInfo) { + /* visible for testing */ static boolean shouldUpdate(AmazonInfo newInfo, AmazonInfo oldInfo) { if (newInfo.getMetadata().isEmpty()) { logger.warn("Newly resolved AmazonInfo is empty, skipping an update cycle"); - return false; } else if (!newInfo.equals(oldInfo)) { int newKeySize = newInfo.getMetadata().size(); int oldKeySize = oldInfo.getMetadata().size(); if (newKeySize < oldKeySize) { logger.warn("Newly resolved AmazonInfo contains less data than previous old:{} -> new:{}, skipping an update cycle", oldKeySize, newKeySize); - return false; + } else { // final case that warrants an update + return true; } } - return true; + return false; } } diff --git a/eureka-client/src/test/java/com/netflix/appinfo/CloudInstanceConfigTest.java b/eureka-client/src/test/java/com/netflix/appinfo/CloudInstanceConfigTest.java index 344587e388..ab355fab4b 100644 --- a/eureka-client/src/test/java/com/netflix/appinfo/CloudInstanceConfigTest.java +++ b/eureka-client/src/test/java/com/netflix/appinfo/CloudInstanceConfigTest.java @@ -67,4 +67,39 @@ public void testResolveDefaultAddress() { config.info.getMetadata().remove(localIpv4.getName()); assertThat(config.resolveDefaultAddress(), is(dummyDefault)); } + + @Test + public void testAmazonInfoUpdate() { + AmazonInfo oldInfo = (AmazonInfo) instanceInfo.getDataCenterInfo(); + + // false for update if equal + AmazonInfo newInfo1 = oldInfo; + assertThat(CloudInstanceConfig.shouldUpdate(newInfo1, oldInfo), is(false)); + + // false for update if new is empty + AmazonInfo newInfo2 = new AmazonInfo(); + assertThat(CloudInstanceConfig.shouldUpdate(newInfo2, oldInfo), is(false)); + + AmazonInfo newInfo3 = new AmazonInfo(); + for (String key : oldInfo.getMetadata().keySet()) { + newInfo3.getMetadata().put(key, oldInfo.getMetadata().get(key)); + } + int originalInfo3Size = newInfo3.getMetadata().size(); + + // true for update if new contains diff data + newInfo3.getMetadata().put(AmazonInfo.MetaDataKey.instanceId.getName(), "foo"); + assertThat(CloudInstanceConfig.shouldUpdate(newInfo3, oldInfo), is(true)); + + // true for update if new contains more data + String newKey = "someNewKey"; + newInfo3.getMetadata().put(newKey, "bar"); + assertThat(newInfo3.getMetadata().size(), is(originalInfo3Size + 1)); + assertThat(CloudInstanceConfig.shouldUpdate(newInfo3, oldInfo), is(true)); + + // false if there is now less data + newInfo3.getMetadata().remove(newKey); + newInfo3.getMetadata().remove(AmazonInfo.MetaDataKey.instanceId.getName()); + assertThat(newInfo3.getMetadata().size(), is(originalInfo3Size - 1)); + assertThat(CloudInstanceConfig.shouldUpdate(newInfo3, oldInfo), is(false)); + } }