Remove[MQB]: isFirstLeaderAdvisory (#835)

Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>

Author: dorjesinpo

src/groups/mqb/mqbblp/mqbblp_clusterorchestrator.cpp

@@ -1539,34 +1539,6 @@ void ClusterOrchestrator::processPrimaryStatusAdvisory(
             return;  // RETURN
         }
     }
-    else if (!d_stateManager_mp->isFirstLeaderAdvisory()) {
-        // Self node has heard from the leader at least once.  Perform
-        // additional validations.
-
-        if (pinfo.primaryNode() != source) {
-            BALL_LOG_ERROR << d_clusterData_p->identity().description()
-                           << ": Partition [" << primaryAdv.partitionId()
-                           << "]: received primary status advisory: "
-                           << primaryAdv
-                           << " from: " << source->nodeDescription()
-                           << ", but current primary is: "
-                           << (pinfo.primaryNode()
-                                   ? pinfo.primaryNode()->nodeDescription()
-                                   : "** null **");
-            return;  // RETURN
-        }
-
-        if (pinfo.primaryLeaseId() != primaryAdv.primaryLeaseId()) {
-            BALL_LOG_ERROR << d_clusterData_p->identity().description()
-                           << ": Partition [" << primaryAdv.partitionId()
-                           << "]: received primary status advisory: "
-                           << primaryAdv << " from perceived primary: "
-                           << source->nodeDescription()
-                           << ", but with different leaseId. Self perceived "
-                           << "leaseId: " << pinfo.primaryLeaseId();
-            return;  // RETURN
-        }
-    }
     else {
         // TODO Remove `mqbi::ClusterStateManager::setPrimary()` when this code
         // is removed.
@@ -1577,23 +1549,35 @@ void ClusterOrchestrator::processPrimaryStatusAdvisory(
         // and hasn't heard from the leader, but various primary nodes have
         // sent their status advisory messages to it.
 
-        // Note that we cannot use self node's status in place of
-        // 'isFirstLeaderAdvisory', because a node may transition from
-        // STARTING to AVAILABLE, but still may not have heard from the leader.
-
-        // Also note that self node could be receiving the 2nd primary status
-        // advisory from the 'source' (recall that a primary sends status
-        // advisory when it sees a new node transitioning to STARTING and again
-        // when transitioning to AVAILABLE), but self node may not have yet
-        // heard from the leader.  So if self's cluster state is already
-        // up-to-date with this primary's status, we don't assert certain
-        // things.
-
         // TBD: Since we are updating cluster state based on a message from the
         // non-leader node, we are breaking the contract that only leader
         // issues writes to the cluster state.  This needs to be reviewed.  See
         // 'StorageMgr::processPrimaryStatusAdvisoryDispatched' as well.
 
+        if (pinfo.primaryNode()) {
+            if (pinfo.primaryNode() != source) {
+                BALL_LOG_ERROR
+                    << d_clusterData_p->identity().description()
+                    << ": Partition [" << primaryAdv.partitionId()
+                    << "]: received primary status advisory: " << primaryAdv
+                    << " from: " << source->nodeDescription()
+                    << ", but current primary is: "
+                    << pinfo.primaryNode()->nodeDescription();
+                return;  // RETURN
+            }
+
+            if (pinfo.primaryLeaseId() != primaryAdv.primaryLeaseId()) {
+                BALL_LOG_ERROR
+                    << d_clusterData_p->identity().description()
+                    << ": Partition [" << primaryAdv.partitionId()
+                    << "]: received primary status advisory: " << primaryAdv
+                    << " from perceived primary: " << source->nodeDescription()
+                    << ", but with different leaseId. Self perceived "
+                    << "leaseId: " << pinfo.primaryLeaseId();
+                return;  // RETURN
+            }
+        }
+
         BALL_LOG_WARN << d_clusterData_p->identity().description()
                       << " Partition [" << primaryAdv.partitionId()
                       << "]: received primary status advisory: " << primaryAdv

src/groups/mqb/mqbblp/mqbblp_clusterstatemanager.cpp

@@ -641,8 +641,6 @@ void ClusterStateManager::onClusterLeader(
 
     if (!node) {
         BSLS_ASSERT_SAFE(mqbc::ElectorInfoLeaderStatus::e_UNDEFINED == status);
-
-        d_isFirstLeaderAdvisory = true;
     }
 }
 
@@ -677,8 +675,6 @@ void ClusterStateManager::onPartitionPrimaryAssignment(
                                                     oldPrimary,
                                                     oldLeaseId);
 
-    d_isFirstLeaderAdvisory = false;
-
     d_afterPartitionPrimaryAssignmentCb(partitionId, primary, status);
 }
 
@@ -699,7 +695,6 @@ ClusterStateManager::ClusterStateManager(
 , d_state_p(clusterState)
 , d_clusterStateLedger_mp(clusterStateLedger)
 , d_storageManager_p(0)
-, d_isFirstLeaderAdvisory(true)
 {
     // executed by *ANY* thread
 
@@ -910,9 +905,6 @@ void ClusterStateManager::sendClusterState(
     BSLS_ASSERT_SAFE(mqbnet::ElectorState::e_LEADER ==
                      d_clusterData_p->electorInfo().electorState());
 
-    // Self is leader and has published advisory above, so update it.
-    d_isFirstLeaderAdvisory = false;
-
     mqbc::ClusterUtil::sendClusterState(d_clusterData_p,
                                         d_clusterStateLedger_mp.get(),
                                         d_storageManager_p,

src/groups/mqb/mqbblp/mqbblp_clusterstatemanager.h

@@ -151,8 +151,6 @@ class ClusterStateManager BSLS_KEYWORD_FINAL
 
     mqbi::StorageManager* d_storageManager_p;
 
-    bool d_isFirstLeaderAdvisory;
-
     AfterPartitionPrimaryAssignmentCb d_afterPartitionPrimaryAssignmentCb;
 
   private:
@@ -486,9 +484,6 @@ class ClusterStateManager BSLS_KEYWORD_FINAL
     void onNodeStopped() BSLS_KEYWORD_OVERRIDE;
 
     // ACCESSORS
-    //   (virtual: mqbi::ClusterStateManager)
-    bool isFirstLeaderAdvisory() const BSLS_KEYWORD_OVERRIDE;
-
     /// Return the cluster state managed by this instacne.
     const mqbc::ClusterState* clusterState() const BSLS_KEYWORD_OVERRIDE;
 
@@ -550,12 +545,6 @@ inline void ClusterStateManager::setAfterPartitionPrimaryAssignmentCb(
 }
 
 // ACCESSORS
-//   (virtual: mqbi::ClusterStateManager)
-inline bool ClusterStateManager::isFirstLeaderAdvisory() const
-{
-    return d_isFirstLeaderAdvisory;
-}
-
 inline const mqbc::ClusterState* ClusterStateManager::clusterState() const
 {
     return d_state_p;

src/groups/mqb/mqbc/mqbc_clusterstatemanager.h

@@ -599,9 +599,6 @@ class ClusterStateManager BSLS_KEYWORD_FINAL
         BSLS_KEYWORD_OVERRIDE;
 
     // ACCESSORS
-    //   (virtual: mqbi::ClusterStateManager)
-    bool isFirstLeaderAdvisory() const BSLS_KEYWORD_OVERRIDE;
-
     /// Return the cluster state managed by this instacne.
     const mqbc::ClusterState* clusterState() const BSLS_KEYWORD_OVERRIDE;
 
@@ -663,13 +660,6 @@ inline void ClusterStateManager::setAfterPartitionPrimaryAssignmentCb(
 
 // ACCESSORS
 //   (virtual: mqbi::ClusterStateManager)
-inline bool ClusterStateManager::isFirstLeaderAdvisory() const
-{
-    BSLS_ASSERT_SAFE(false && "NOT IMPLEMENTED!");
-
-    return false;
-}
-
 inline const mqbc::ClusterState* ClusterStateManager::clusterState() const
 {
     return d_state_p;

src/groups/mqb/mqbi/mqbi_clusterstatemanager.h

@@ -312,8 +312,6 @@ class ClusterStateManager {
     virtual void onNodeStopped() = 0;
 
     // ACCESSORS
-    virtual bool isFirstLeaderAdvisory() const = 0;
-
     /// Return the cluster state managed by this instacne.
     virtual const mqbc::ClusterState* clusterState() const = 0;