Merge pull request #1569 from OlgaMaciaszek/2.x-check-current-instance-status-before-update

Check current instance status before update.

Author: howardyuan

.github/workflows/nebula-ci.yml

@@ -21,14 +21,14 @@ jobs:
         uses: actions/setup-java@v1
         with:
           java-version: ${{ matrix.java }}
-      - uses: actions/cache@v1
+      - uses: actions/cache@v4
         id: gradle-cache
         with:
           path: ~/.gradle/caches
           key: ${{ runner.os }}-gradle-${{ hashFiles('**/gradle/dependency-locks/*.lockfile') }}
           restore-keys: |
             - ${{ runner.os }}-gradle-
-      - uses: actions/cache@v1
+      - uses: actions/cache@v4
         id: gradle-wrapper-cache
         with:
           path: ~/.gradle/wrapper

.github/workflows/nebula-publish.yml

@@ -17,14 +17,14 @@ jobs:
         uses: actions/setup-java@v1
         with:
           java-version: 11
-      - uses: actions/cache@v1
+      - uses: actions/cache@v4
         id: gradle-cache
         with:
           path: ~/.gradle/caches
           key: ${{ runner.os }}-gradle-${{ hashFiles('**/gradle/dependency-locks/*.lockfile') }}
           restore-keys: |
             - ${{ runner.os }}-gradle-
-      - uses: actions/cache@v1
+      - uses: actions/cache@v4
         id: gradle-wrapper-cache
         with:
           path: ~/.gradle/wrapper

.github/workflows/nebula-snapshot.yml

@@ -17,13 +17,13 @@ jobs:
         uses: actions/setup-java@v1
         with:
           java-version: 11
-      - uses: actions/cache@v2
+      - uses: actions/cache@v4
         id: gradle-cache
         with:
           path: |
             ~/.gradle/caches
           key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
-      - uses: actions/cache@v2
+      - uses: actions/cache@v4
         id: gradle-wrapper-cache
         with:
           path: |

eureka-client/src/main/java/com/netflix/appinfo/ApplicationInfoManager.java

@@ -41,7 +41,7 @@
  * </p>
  *
  *
- * @author Karthik Ranganathan, Greg Kim
+ * @author Karthik Ranganathan, Greg Kim, Olga Maciaszek-Sharma
  *
  */
 @Singleton
@@ -171,13 +171,29 @@ public synchronized void setInstanceStatus(InstanceStatus status) {
 
         InstanceStatus prev = instanceInfo.setStatus(next);
         if (prev != null) {
-            for (StatusChangeListener listener : listeners.values()) {
-                try {
-                    listener.notify(new StatusChangeEvent(prev, next));
-                } catch (Exception e) {
-                    logger.warn("failed to notify listener: {}", listener.getId(), e);
-                }
-            }
+            notifyListeners(prev, next);
+        }
+    }
+
+    /**
+     * Set the status of this instance only if the current status equals expected status
+     * in order to avoid concurrent modification.
+     * Application can use this to indicate whether it is ready to receive traffic.
+     * Setting the status here also notifies all registered listeners
+     * of a status change event.
+     *
+     * @param expectedStatus expected current Status of the instance
+     * @param updateStatus Status of the instance to be set
+     */
+    public synchronized void setInstanceStatus(InstanceStatus expectedStatus, InstanceStatus updateStatus) {
+        InstanceStatus next = instanceStatusMapper.map(updateStatus);
+        if (next == null) {
+            return;
+        }
+
+        InstanceStatus prev = instanceInfo.setStatus(expectedStatus, next);
+        if (prev != null) {
+            notifyListeners(prev, next);
         }
     }
 
@@ -261,6 +277,17 @@ public void refreshLeaseInfoIfRequired() {
         }
     }
 
+    private void notifyListeners(InstanceStatus prev, InstanceStatus next) {
+        for (StatusChangeListener listener : listeners.values()) {
+            try {
+                listener.notify(new StatusChangeEvent(prev, next));
+            }
+            catch (Exception e) {
+                logger.warn("failed to notify listener: {}", listener.getId(), e);
+            }
+        }
+    }
+
     public static interface StatusChangeListener {
         String getId();
 

eureka-client/src/main/java/com/netflix/appinfo/InstanceInfo.java

@@ -29,7 +29,6 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonRootName;
 import com.netflix.appinfo.providers.Archaius1VipAddressResolver;
-import com.netflix.appinfo.providers.EurekaConfigBasedInstanceInfoProvider;
 import com.netflix.appinfo.providers.VipAddressResolver;
 import com.netflix.discovery.converters.Auto;
 import com.netflix.discovery.converters.EurekaJacksonCodec.InstanceInfoSerializer;
@@ -48,7 +47,7 @@
  * serialized as specified by the <code>@Serializer</code>.
  * </p>
  *
- * @author Karthik Ranganathan, Greg Kim
+ * @author Karthik Ranganathan, Greg Kim, Olga Maciaszek-Sharma
  */
 @Serializer("com.netflix.discovery.converters.EntityBodyConverter")
 @XStreamAlias("instance")
@@ -1173,6 +1172,21 @@ public synchronized InstanceStatus setStatus(InstanceStatus status) {
         return null;
     }
 
+    /**
+     * Set the status for this instance only when the current status matches the expected status.
+     *
+     * @param expected status for this instance.
+     * @param status status to set for this instance.
+     * @return the prev status if a different status from the current was set, null otherwise
+     */
+    public synchronized InstanceStatus setStatus(InstanceStatus expected, InstanceStatus status) {
+        InstanceStatus prev = this.status;
+        if (expected == null || !expected.equals(prev)) {
+            return null;
+        }
+        return setStatus(status);
+    }
+
     /**
      * Set the status for this instance without updating the dirty timestamp.
      *

eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java

@@ -1380,16 +1380,17 @@ void refreshInstanceInfo() {
         applicationInfoManager.refreshDataCenterInfoIfRequired();
         applicationInfoManager.refreshLeaseInfoIfRequired();
 
+        InstanceStatus current = instanceInfo.getStatus();
         InstanceStatus status;
         try {
-            status = getHealthCheckHandler().getStatus(instanceInfo.getStatus());
+            status = getHealthCheckHandler().getStatus(current);
         } catch (Exception e) {
             logger.warn("Exception from healthcheckHandler.getStatus, setting status to DOWN", e);
             status = InstanceStatus.DOWN;
         }
 
         if (null != status) {
-            applicationInfoManager.setInstanceStatus(status);
+            applicationInfoManager.setInstanceStatus(current, status);
         }
     }
 

eureka-client/src/test/java/com/netflix/appinfo/InstanceInfoUnitTest.java

@@ -0,0 +1,43 @@
+package com.netflix.appinfo;
+
+import com.netflix.discovery.util.InstanceInfoGenerator;
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author Olga Maciaszek-Sharma
+ */
+public class InstanceInfoUnitTest {
+
+	@Test
+	public void testInstanceStatusUpdated() {
+		InstanceInfo instanceInfo = InstanceInfoGenerator.takeOne();
+
+		instanceInfo.setStatus(InstanceInfo.InstanceStatus.UP,
+				InstanceInfo.InstanceStatus.OUT_OF_SERVICE);
+
+		assertThat(instanceInfo.getStatus()).isEqualTo(InstanceInfo.InstanceStatus.OUT_OF_SERVICE);
+	}
+
+	@Test
+	public void testInstanceStatusNotUpdatedWhenExpectedStatusNotMatched() {
+		InstanceInfo instanceInfo = InstanceInfoGenerator.takeOne();
+
+		instanceInfo.setStatus(InstanceInfo.InstanceStatus.DOWN,
+				InstanceInfo.InstanceStatus.OUT_OF_SERVICE);
+
+		assertThat(instanceInfo.getStatus()).isEqualTo(InstanceInfo.InstanceStatus.UP);
+	}
+
+	@Test
+	public void testInstanceStatusNotUpdatedWhenExpectedStatusNull() {
+		InstanceInfo instanceInfo = InstanceInfoGenerator.takeOne();
+
+		instanceInfo.setStatus(null,
+				InstanceInfo.InstanceStatus.OUT_OF_SERVICE);
+
+		assertThat(instanceInfo.getStatus()).isEqualTo(InstanceInfo.InstanceStatus.UP);
+	}
+
+}