Add support for 'isFSMWorkflow' flag and fix FSM workflow #161

kaikulimu · 2023-11-29T21:30:56Z

Add support for FSM workflow of cluster node startup sequence

Note:
70beb20 adopts Vitaly's #174 into this PR
751032b adopts Vitaly's #190 into this PR

src/groups/mqb/mqbs/mqbs_storageutil.h

src/groups/mqb/mqbs/mqbs_filestoreutil.cpp

src/groups/mqb/mqbblp/mqbblp_clusterorchestrator.cpp

src/groups/mqb/mqbc/mqbc_clusterfsm.cpp

src/groups/mqb/mqbc/mqbc_incoreclusterstateledger.cpp

src/groups/mqb/mqbc/mqbc_partitionfsm.cpp

src/groups/mqb/mqbc/mqbc_recoverymanager.cpp

src/groups/mqb/mqbc/mqbc_storagemanager.cpp

kaikulimu · 2023-12-04T22:53:03Z

@678098 Addressed your comments. Can review again.

kaikulimu · 2023-12-14T20:49:16Z

@dorjesinpo is going to make some changes near StorageUtil::recoveredQueuesCb() to retrieve the config of strong vs eventual consistency mode. This might potentially conflict the FSM PR so we will have to be careful.

kaikulimu · 2023-12-15T16:17:46Z

@678098's work on purge queue command can potentially conflict with this PR, too. Must be careful.

dorjesinpo

Not a thorough review (quarter-note has reviewed before)
Some minor notes

dorjesinpo · 2024-03-25T15:35:26Z

src/groups/mqb/mqbc/mqbc_clusterdata.cpp

@@ -7,8 +7,8 @@
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
+// Unless required byClusterDataIdentityapplicable law or agreed to in writing,


Unless required by applicable law ...

dorjesinpo · 2024-03-25T17:18:07Z

src/groups/mqb/mqbblp/mqbblp_cluster.cpp

@@ -3605,8 +3628,6 @@ Cluster::sendRequest(const Cluster::RequestManagerType::RequestSp& request,
                     mqbnet::ClusterNode*                          target,
                     bsls::TimeInterval                            timeout)
 {
-    // executed by the cluster *DISPATCHER* thread


Is that not in the Dispatcher thread anymore? If so, need to comment on the thread safety

Yes, it is now possible to be invoked in the queue dispatcher thread by mqbc::StorageManager:

blazingmq/src/groups/mqb/mqbc/mqbc_storagemanager.cpp

Line 1742 in 751032b

bmqt::GenericResult::Enum status = d_clusterData_p->cluster()->sendRequest(

ElectorInfo::setElectorInfo is "executed by the cluster DISPATCHER thread" so we need to put some warning about thread safety. We consider the access to d_leaderNode_p (and everything else here) thread-safe.

Added some comment about thread safety. Importantly, Cluster::sendRequest() has been invoked in queue dispatcher threads since before FSM, for example at RecoveryManager::onPartitionSyncStateQueryResponseDispatched().

dorjesinpo · 2024-03-25T18:38:17Z

src/groups/mqb/mqbc/mqbc_clusterdata.cpp

-    if (bdls::ProcessUtil::getProcessName(&(identity.processName())) != 0) {
-        identity.processName() = "** UNKNOWN **";
+    bmqp_ctrlmsg::ClientIdentity identity(allocator);
+    if (!isRemote) {


Is there no need for proxy detailed identity?

If you look at eb05973 where this change is made, you will see that the original code was:

, d_identity(cluster->isRemote() ? ClusterDataIdentity(name, bmqp_ctrlmsg::ClientIdentity(allocator), allocator) : clusterIdentity(name, d_membership.netCluster()))

Hence, if we are a remote node, an empty bmqp_ctrlmsg::ClientIdentity is passed in.

I merely refactored and simplified the code.

dorjesinpo · 2024-03-25T19:11:41Z

src/groups/mqb/mqbc/mqbc_clusterutil.cpp

@@ -785,6 +798,8 @@ void ClusterUtil::populateQueueAssignmentAdvisory(
    BSLS_ASSERT_SAFE(domain);
    BSLS_ASSERT_SAFE(clusterData->electorInfo().isSelfActiveLeader());

+    BALL_LOG_SET_CATEGORY(k_LOG_CATEGORY);


Do we need it (considering BALL_LOG_SET_CLASS_CATEGORY("MQBC.CLUSTERUTIL");)?
And below

Good catch, these lines are redundant. Will remove them.

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

kaikulimu · 2024-03-26T20:56:40Z

@dorjesinpo Ready for review again

kaikulimu requested a review from quarter-note November 29, 2023 21:30

kaikulimu assigned quarter-note Nov 29, 2023

kaikulimu requested a review from a team as a code owner November 29, 2023 21:30

kaikulimu force-pushed the isFSM branch 3 times, most recently from 6bf3068 to 7a4ca95 Compare November 30, 2023 16:56

678098 reviewed Dec 1, 2023

View reviewed changes

kaikulimu assigned kaikulimu and unassigned quarter-note Dec 4, 2023

kaikulimu force-pushed the isFSM branch from 71e9d51 to c06719d Compare December 4, 2023 22:50

kaikulimu assigned 678098 and unassigned kaikulimu Dec 4, 2023

kaikulimu force-pushed the isFSM branch 2 times, most recently from 02ad95b to f7af3f4 Compare December 26, 2023 22:41

kaikulimu force-pushed the isFSM branch 5 times, most recently from 9c029f6 to fc31a38 Compare March 6, 2024 19:11

kaikulimu force-pushed the isFSM branch 8 times, most recently from 4cc4d4a to 7bb2371 Compare March 8, 2024 19:25

kaikulimu force-pushed the isFSM branch 4 times, most recently from 9578dc8 to c8b9092 Compare March 15, 2024 19:22

kaikulimu force-pushed the isFSM branch from c8b9092 to 751032b Compare March 25, 2024 14:08

dorjesinpo reviewed Mar 25, 2024

View reviewed changes

kaikulimu force-pushed the isFSM branch from 89fd369 to 6e29ec2 Compare March 26, 2024 18:22

kaikulimu added 13 commits March 26, 2024 14:23

Add support for 'isFSMWorkflow' flag and fix FSM workflow

4df09ee

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

FSM workflow: Fix compile errors

81dd162

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

typedef DomainQueueMessagesCountMaps: mqbs::StorageUtil -> mqbc

f6cf704

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

PR#161: Address comments

8b62482

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

FSM workflow: Adopt PR#174 'fix: mix of strong and weak consistencies'

212ec64

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

blp::Cluster: Add back missing control message processing types

76e74c7

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

test_leader_node_delay.py: Fix log grepping post-FSM

f870932

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

run-tests: Also run integration tests in FSM mode

46cccb2

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

mqbc::ClusterDataIdentity: Simplify the code

3760d47

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

blp::ClusterProxy, mqbmock::Cluster: Use description from ClusterData

9a8b062

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

mqbc::ClusterData: Incorporate mqbcfg::ClusterProxyDefinition

b14656a

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

FSM workflow: Adopt PR#190 'Application subscriptions'

e478222

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

PR#161: Address comments

fdf512e

Signed-off-by: Yuan Jing Vincent Yan <[email protected]>

kaikulimu force-pushed the isFSM branch from 6e29ec2 to fdf512e Compare March 26, 2024 18:23

kaikulimu assigned dorjesinpo and unassigned 678098 Mar 26, 2024

dorjesinpo approved these changes Mar 26, 2024

View reviewed changes

dorjesinpo merged commit 1de0b4e into bloomberg:main Mar 26, 2024
10 checks passed

kaikulimu deleted the isFSM branch April 10, 2024 15:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for 'isFSMWorkflow' flag and fix FSM workflow #161

Add support for 'isFSMWorkflow' flag and fix FSM workflow #161

kaikulimu commented Nov 29, 2023 •

edited

Loading

kaikulimu commented Dec 4, 2023

kaikulimu commented Dec 14, 2023

kaikulimu commented Dec 15, 2023

dorjesinpo left a comment

dorjesinpo Mar 25, 2024

kaikulimu Mar 26, 2024 •

edited

Loading

dorjesinpo Mar 25, 2024

kaikulimu Mar 26, 2024

dorjesinpo Mar 26, 2024

kaikulimu Mar 26, 2024

dorjesinpo Mar 25, 2024

kaikulimu Mar 26, 2024 •

edited

Loading

kaikulimu Mar 26, 2024

dorjesinpo Mar 25, 2024

kaikulimu Mar 26, 2024

kaikulimu commented Mar 26, 2024

Add support for 'isFSMWorkflow' flag and fix FSM workflow #161

Add support for 'isFSMWorkflow' flag and fix FSM workflow #161

Conversation

kaikulimu commented Nov 29, 2023 • edited Loading

kaikulimu commented Dec 4, 2023

kaikulimu commented Dec 14, 2023

kaikulimu commented Dec 15, 2023

dorjesinpo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikulimu Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikulimu Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikulimu commented Mar 26, 2024

kaikulimu commented Nov 29, 2023 •

edited

Loading

kaikulimu Mar 26, 2024 •

edited

Loading

kaikulimu Mar 26, 2024 •

edited

Loading