fix secondary list are not respected

Author: BewareMyPower

lib/AutoClusterFailover.cc

@@ -258,30 +258,36 @@ class AutoClusterFailoverImpl : public std::enable_shared_from_this<AutoClusterF
         onServiceInfoUpdate_(current());
     }
 
-    void probeSecondaryFrom(size_t index, CompletionCallback done) {
+    void probeSecondaryFrom(size_t index, const ServiceInfo* excludedServiceInfo, ProbeCallback callback) {
         if (index >= config_.secondary.size()) {
-            done();
+            callback(false);
+            return;
+        }
+
+        if (&config_.secondary[index] == excludedServiceInfo) {
+            probeSecondaryFrom(index + 1, excludedServiceInfo, std::move(callback));
             return;
         }
 
         auto weakSelf = weak_from_this();
-        probeAvailableAsync(config_.secondary[index],
-                            [weakSelf, index, done = std::move(done)](bool available) mutable {
-                                auto self = weakSelf.lock();
-                                if (!self) {
-                                    return;
-                                }
-
-                                LOG_DEBUG("Detected secondary " << self->config_.secondary[index].serviceUrl()
-                                                                << " availability: " << available);
-                                if (available) {
-                                    self->switchTo(&self->config_.secondary[index]);
-                                    done();
-                                    return;
-                                }
-
-                                self->probeSecondaryFrom(index + 1, std::move(done));
-                            });
+        probeAvailableAsync(
+            config_.secondary[index],
+            [weakSelf, index, excludedServiceInfo, callback = std::move(callback)](bool available) mutable {
+                auto self = weakSelf.lock();
+                if (!self) {
+                    return;
+                }
+
+                LOG_DEBUG("Detected secondary " << self->config_.secondary[index].serviceUrl()
+                                                << " availability: " << available);
+                if (available) {
+                    self->switchTo(&self->config_.secondary[index]);
+                    callback(true);
+                    return;
+                }
+
+                self->probeSecondaryFrom(index + 1, excludedServiceInfo, std::move(callback));
+            });
     }
 
     void checkAndFailoverToSecondaryAsync(CompletionCallback done) {
@@ -305,10 +311,45 @@ class AutoClusterFailoverImpl : public std::enable_shared_from_this<AutoClusterF
                 return;
             }
 
-            self->probeSecondaryFrom(0, std::move(done));
+            self->probeSecondaryFrom(0, nullptr, [done = std::move(done)](bool) mutable { done(); });
         });
     }
 
+    void failoverFromUnavailableSecondaryAsync(CompletionCallback done) {
+        auto weakSelf = weak_from_this();
+        probeAvailableAsync(
+            config_.primary, [weakSelf, done = std::move(done)](bool primaryAvailable) mutable {
+                auto self = weakSelf.lock();
+                if (!self) {
+                    return;
+                }
+
+                LOG_DEBUG("Detected primary while secondary is unavailable "
+                          << self->config_.primary.serviceUrl() << " availability: " << primaryAvailable);
+                if (primaryAvailable) {
+                    self->switchTo(&self->config_.primary);
+                    done();
+                    return;
+                }
+
+                self->probeSecondaryFrom(
+                    0, self->currentServiceInfo_,
+                    [weakSelf, done = std::move(done)](bool switchedToAnotherSecondary) mutable {
+                        auto self = weakSelf.lock();
+                        if (!self) {
+                            return;
+                        }
+
+                        if (switchedToAnotherSecondary) {
+                            done();
+                            return;
+                        }
+
+                        self->checkSwitchBackToPrimaryAsync(std::move(done), false);
+                    });
+            });
+    }
+
     void checkSwitchBackToPrimaryAsync(CompletionCallback done, std::optional<bool> primaryAvailableHint) {
         auto handlePrimaryAvailable = [weakSelf = weak_from_this(),
                                        done = std::move(done)](bool primaryAvailable) mutable {
@@ -358,23 +399,7 @@ class AutoClusterFailoverImpl : public std::enable_shared_from_this<AutoClusterF
                 return;
             }
 
-            self->probeAvailableAsync(
-                self->config_.primary, [weakSelf, done = std::move(done)](bool primaryAvailable) mutable {
-                    auto self = weakSelf.lock();
-                    if (!self) {
-                        return;
-                    }
-
-                    LOG_DEBUG("Detected primary while secondary is unavailable "
-                              << self->config_.primary.serviceUrl() << " availability: " << primaryAvailable);
-                    if (primaryAvailable) {
-                        self->switchTo(&self->config_.primary);
-                        done();
-                        return;
-                    }
-
-                    self->checkSwitchBackToPrimaryAsync(std::move(done), false);
-                });
+            self->failoverFromUnavailableSecondaryAsync(std::move(done));
         });
     }
 };

tests/ServiceInfoProviderTest.cc

@@ -273,6 +273,44 @@ TEST(AutoClusterFailoverTest, testSwitchBackToPrimaryAfterRecoveryDelay) {
     ASSERT_EQ(updates[2], primaryUrl);
 }
 
+TEST(AutoClusterFailoverTest, testFailoverToAnotherSecondaryWhenCurrentSecondaryIsUnavailable) {
+    ProbeTcpServer primary;
+    const auto primaryUrl = primary.getServiceUrl();
+    primary.stop();
+
+    ProbeTcpServer firstSecondary;
+    const auto firstSecondaryUrl = firstSecondary.getServiceUrl();
+
+    ProbeTcpServer secondSecondary;
+    const auto secondSecondaryUrl = secondSecondary.getServiceUrl();
+
+    ServiceUrlObserver observer;
+    AutoClusterFailover provider =
+        AutoClusterFailover::Builder(ServiceInfo(primaryUrl),
+                                     {ServiceInfo(firstSecondaryUrl), ServiceInfo(secondSecondaryUrl)})
+            .withCheckInterval(20ms)
+            .withFailoverThreshold(4)
+            .withSwitchBackThreshold(6)
+            .build();
+
+    observer.onUpdate(provider.initialServiceInfo());
+    provider.initialize([&observer](const ServiceInfo &serviceInfo) { observer.onUpdate(serviceInfo); });
+
+    ASSERT_TRUE(
+        waitUntil(2s, [&observer, &firstSecondaryUrl] { return observer.last() == firstSecondaryUrl; }));
+
+    firstSecondary.stop();
+
+    ASSERT_TRUE(
+        waitUntil(2s, [&observer, &secondSecondaryUrl] { return observer.last() == secondSecondaryUrl; }));
+
+    const auto updates = observer.snapshot();
+    ASSERT_EQ(updates.size(), 3u);
+    ASSERT_EQ(updates[0], primaryUrl);
+    ASSERT_EQ(updates[1], firstSecondaryUrl);
+    ASSERT_EQ(updates[2], secondSecondaryUrl);
+}
+
 TEST(ServiceInfoProviderTest, testSwitchCluster) {
     extern std::string getToken();  // from tests/AuthTokenTest.cc
     // Access "private/auth" namespace in cluster 1