Skip to content

Feat FSM mode: Still write to QList file #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

kaikulimu
Copy link
Collaborator

@kaikulimu kaikulimu commented Mar 18, 2025

In the existing code, FSM mode never writes to QList file. In this PR, we add a new doesFSMWriteQLIST flag to determine whether FSM mode still writes to QList file. Note that the PartitionFSMs do not synchronize QList file; they only synchronize journal and data files. Such synchronization will be added in a subsequent PR.

A benefit of this PR is that we are finally able to test switching from non-FSM to FSM then back to non-FSM in test_restart.py.

Also note that this PR is low-risk since it only affects FSM mode.

@kaikulimu kaikulimu force-pushed the fsm-test--fsm-write-qlist branch 2 times, most recently from 1af2464 to 4e9dbfc Compare March 24, 2025 14:31
@kaikulimu kaikulimu force-pushed the fsm-test--fsm-write-qlist branch from 9ac9f23 to c7290fd Compare March 28, 2025 18:48
@kaikulimu kaikulimu changed the title Fsm test fsm write qlist Feat FSM mode: Still write to QList file Mar 28, 2025
@kaikulimu kaikulimu force-pushed the fsm-test--fsm-write-qlist branch 2 times, most recently from be8a299 to 0853597 Compare March 28, 2025 19:18
@kaikulimu kaikulimu requested a review from dorjesinpo March 28, 2025 19:31
@kaikulimu kaikulimu marked this pull request as ready for review March 28, 2025 19:32
@kaikulimu kaikulimu requested a review from a team as a code owner March 28, 2025 19:32
@kaikulimu kaikulimu force-pushed the fsm-test--fsm-write-qlist branch 3 times, most recently from 946ac16 to 339a417 Compare March 28, 2025 19:46
@kaikulimu kaikulimu force-pushed the fsm-test--fsm-write-qlist branch from 339a417 to c8234b1 Compare April 8, 2025 18:08
@kaikulimu kaikulimu force-pushed the fsm-test--fsm-write-qlist branch from c8234b1 to 2814c97 Compare April 8, 2025 19:03
Copy link
Collaborator

@dorjesinpo dorjesinpo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix those IT failures

@@ -246,6 +246,10 @@ class RecoveryManager {
/// Allocator to use
bslma::Allocator* d_allocator_p;

bool d_qListAware;
// Whether the broker still read and writes to the to-be-deprecated Qlist
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readS

if (needQList) {
recoveryFileSet.setQlistFileSize(d_config.maxQlistFileSize());
}
recoveryFileSet.setQlistFileSize(d_qListAware ? d_config.maxQlistFileSize()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not if (d_qListAware) { ?

if (needQList) {
fileSetSp->d_qlistFilePosition = qlistFileOffset;
}
fileSetSp->d_qlistFilePosition = d_qListAware ? qlistFileOffset : 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not if (d_qListAware) { ? (and below)

<< BMQTSK_ALARMLOG_END;
return rc_QUEUE_URI_MISMATCH; // RETURN
if (d_isFSMWorkflow) {
if (qinfo.canonicalQueueUri() != uri) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If d_isFSMWorkflow is false, would qinfo.canonicalQueueUri() ever be not empty?

<< ", index: " << jit->recordIndex()
<< ", with appId/appKey mismatch. "
"Expected appId/appKey ["
<< qinfoAppCit->second << ", "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If qinfoAppCit == qinfo.appIdKeyPairs().cend(), dereferencing qinfoAppCit would not work.

@@ -3880,6 +3914,10 @@ int FileStore::issueSyncPointInternal(SyncPointType::Enum type,
2 * FileStoreProtocol::k_JOURNAL_RECORD_SIZE));
}

BALL_LOG_ERROR << "Henshin! d_primaryLeaseId: " << d_primaryLeaseId
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Henshin!

@@ -5022,7 +5058,8 @@ void FileStore::replicateRecord(bmqp::StorageMessageType::Enum type,
FileStoreProtocol::k_JOURNAL_RECORD_SIZE);

bdlbb::BlobBuffer dataBlobBuffer;
if (!d_isFSMWorkflow || bmqp::StorageMessageType::e_DATA == type) {
if (bmqp::StorageMessageType::e_DATA == type ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about else? Any asserts?

bmqp::Protocol::k_WORD_SIZE
: 0;

BALL_LOG_ERROR << "Henshin Baron! " << d_qListAware << ", "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Henshin Baron!

@dorjesinpo dorjesinpo assigned kaikulimu and unassigned dorjesinpo Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants