Addressing review

Signed-off-by: dorjesinpo <[email protected]>
bloomberg · Dec 10, 2024 · adae77e · adae77e
1 parent bab30fd
commit adae77e
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 18 deletions.
diff --git a/src/groups/mqb/mqbblp/mqbblp_cluster.cpp b/src/groups/mqb/mqbblp/mqbblp_cluster.cpp
@@ -2210,6 +2210,9 @@ void Cluster::onRecoveryStatusDispatched(
                 BSLS_ASSERT_SAFE(itMp->storage()->partitionId() ==
                                  static_cast<int>(pid));
 
+                // TODO:  wrong thread to call 'loadVirtualStorageDetails'
+                // but 'onRecoveryStatusDispatched' should not be concurrent
+                // with any of 'add/removeVirtualStorage' calls.
                 AppInfos appIdInfos;
                 itMp->storage()->loadVirtualStorageDetails(&appIdInfos);
 

diff --git a/src/groups/mqb/mqbc/mqbc_storageutil.cpp b/src/groups/mqb/mqbc/mqbc_storageutil.cpp
@@ -155,12 +155,12 @@ void StorageUtil::loadAddedAndRemovedEntries(
     loadDifference(removedEntries, existingEntries, newEntries);
 }
 
-bool StorageUtil::loadUpdatedAppInfos(AppInfos* addedAppInfos,
-                                      AppInfos* removedAppInfos,
-                                      const mqbs::ReplicatedStorage& storage,
+bool StorageUtil::loadUpdatedAppInfos(AppInfos*       addedAppInfos,
+                                      AppInfos*       removedAppInfos,
+                                      const AppInfos& existingAppInfos,
                                       const AppInfos& newAppInfos)
 {
-    // executed by the *CLUSTER DISPATCHER* thread
+    // executed by the *QUEUE_DISPATCHER* thread
 
     // PRECONDITIONS
     BSLS_ASSERT_SAFE(addedAppInfos);
@@ -181,9 +181,6 @@ bool StorageUtil::loadUpdatedAppInfos(AppInfos* addedAppInfos,
     // list of newly added and removed appIds, and then invoking 'updateQueue'
     // in the appropriate thread.
 
-    AppInfos existingAppInfos;
-    storage.loadVirtualStorageDetails(&existingAppInfos);
-
     loadAddedAndRemovedEntries(addedAppInfos,
                                removedAppInfos,
                                existingAppInfos,
@@ -294,7 +291,7 @@ void StorageUtil::updateQueuePrimaryDispatched(
 
     bool hasUpdate = loadUpdatedAppInfos(&addedAppInfos,
                                          &removedAppInfos,
-                                         *storage,
+                                         existingAppInfos,
                                          appIdKeyPairs);
     if (!hasUpdate) {
         // No update needed for AppId/Key pairs.

diff --git a/src/groups/mqb/mqbc/mqbc_storageutil.h b/src/groups/mqb/mqbc/mqbc_storageutil.h
@@ -189,14 +189,14 @@ struct StorageUtil {
 
     /// Load into the specified `addedAppInfos` and
     /// `removedAppInfos` the appId/key pairs which have been added and
-    /// removed respectively for the specified `storage` based on the
+    /// removed respectively for the specified `existingAppInfos` based on the
     /// specified `newAppInfos`.  Return true if there are any added or removed
     /// appId/key pairs, false otherwise.
     ///
-    /// THREAD: Executed by the cluster dispatcher thread.
-    static bool loadUpdatedAppInfos(AppInfos* addedAppInfos,
-                                    AppInfos* removedAppInfos,
-                                    const mqbs::ReplicatedStorage& storage,
+    /// THREAD: Executed by the queue dispatcher thread.
+    static bool loadUpdatedAppInfos(AppInfos*       addedAppInfos,
+                                    AppInfos*       removedAppInfos,
+                                    const AppInfos& existingAppInfos,
                                     const AppInfos& newAppInfos);
 
     /// THREAD: Executed by the Queue's dispatcher thread.

diff --git a/src/groups/mqb/mqbi/mqbi_storage.h b/src/groups/mqb/mqbi/mqbi_storage.h
@@ -375,10 +375,7 @@ class Storage {
   public:
     // PUBLIC TYPES
 
-    /// `AppInfo` is an alias for an (appId, appKey) pairing
-    /// representing unique virtual storage identification.
-
-    /// `AppInfos` is an alias for a set of pairs of appId and appKey
+    /// `AppInfos` is an alias for a map [appId] -> appKey
     typedef bsl::unordered_map<bsl::string, mqbu::StorageKey> AppInfos;
 
     typedef bmqc::Array<mqbu::StorageKey,

diff --git a/src/groups/mqb/mqbs/mqbs_datastore.h b/src/groups/mqb/mqbs/mqbs_datastore.h
@@ -272,7 +272,6 @@ class DataStoreConfigQueueInfo {
   public:
     // TYPES
     typedef mqbi::Storage::AppInfos AppInfos;
-    typedef AppInfos::const_iterator AppInfo;
 
   private:
     // DATA