Merge pull request #728 from qiangdavidliu/master

add additional sanity check in shouldRefresh AmazonInfo
Netflix · Jan 5, 2016 · 9d3b315 · 9d3b315
2 parents b4ba5a2 + 675e084
commit 9d3b315
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 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
@@ -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);
  }
  }
+
+ /* 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");
+ } 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);
+ } else { // final case that warrants an update
+ 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
@@ -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));
+ }
 }