Skip to content

Refactor[mqbs::FileSet]: combine data/journal/qlist data in a struct #1510

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.

Sign up for GitHub

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
678098 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactor[mqbs::FileSet]: combine data/journal/qlist data in a struct #1510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/groups/mqb/mqbc/mqbc_recoverymanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,26 +889,27 @@ int RecoveryManager::createRecoveryFileSet(bsl::ostream& errorDescription,
}

RecoveryContext& recoveryCtx = d_recoveryContextVec[partitionId];
recoveryCtx.d_recoveryFileSet.setJournalFile(fileSetSp->d_journalFileName)
recoveryCtx.d_recoveryFileSet
.setJournalFile(fileSetSp->d_journal.d_fileName)
.setJournalFileSize(d_dataStoreConfig.maxJournalFileSize())
.setDataFile(fileSetSp->d_dataFileName)
.setDataFile(fileSetSp->d_data.d_fileName)
.setDataFileSize(d_dataStoreConfig.maxDataFileSize())
.setQlistFileSize(d_dataStoreConfig.maxQlistFileSize())
.setQlistFile(fileSetSp->d_qlistFileName);
.setQlistFile(fileSetSp->d_qlist.d_fileName);

mqbs::FileStoreUtil::loadCurrentFiles(&recoveryCtx.d_recoveryFileSet,
*fileSetSp,
d_qListAware); // needQList

recoveryCtx.d_mappedJournalFd = fileSetSp->d_journalFile;
recoveryCtx.d_journalFilePosition = fileSetSp->d_journalFilePosition;
recoveryCtx.d_mappedDataFd = fileSetSp->d_dataFile;
recoveryCtx.d_dataFilePosition = fileSetSp->d_dataFilePosition;
recoveryCtx.d_mappedJournalFd = fileSetSp->d_journal.d_file;
recoveryCtx.d_journalFilePosition = fileSetSp->d_journal.d_filePosition;
recoveryCtx.d_mappedDataFd = fileSetSp->d_data.d_file;
recoveryCtx.d_dataFilePosition = fileSetSp->d_data.d_filePosition;
BSLS_ASSERT_SAFE(recoveryCtx.d_mappedJournalFd.isValid());
BSLS_ASSERT_SAFE(recoveryCtx.d_mappedDataFd.isValid());
if (d_qListAware) {
recoveryCtx.d_mappedQlistFd = fileSetSp->d_qlistFile;
recoveryCtx.d_qlistFilePosition = fileSetSp->d_qlistFilePosition;
recoveryCtx.d_mappedQlistFd = fileSetSp->d_qlist.d_file;
recoveryCtx.d_qlistFilePosition = fileSetSp->d_qlist.d_filePosition;
BSLS_ASSERT_SAFE(recoveryCtx.d_mappedQlistFd.isValid());
}

Expand All @@ -922,7 +923,7 @@ int RecoveryManager::createRecoveryFileSet(bsl::ostream& errorDescription,
<< ", data file position: " << recoveryCtx.d_dataFilePosition;
if (d_qListAware) {
BALL_LOG_OUTPUT_STREAM << ", qlist file position: "
<< fileSetSp->d_qlistFilePosition;
<< fileSetSp->d_qlist.d_filePosition;
}
}

Expand Down
72 changes: 37 additions & 35 deletions src/groups/mqb/mqbs/mqbs_fileset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#include <bslma_usesbslmaallocator.h>
#include <bslmf_nestedtraitdeclaration.h>
#include <bsls_atomic.h>
#include <bsls_cpp11.h>
#include <bsls_keyword.h>
#include <bsls_types.h>

namespace BloombergLP {
Expand All @@ -53,36 +53,39 @@ class FileStore;
// ==============

/// Value-semantic type representing a set of BlazingMQ data store files.
struct FileSet BSLS_CPP11_FINAL {
struct FileSet BSLS_KEYWORD_FINAL {
public:
// PUBLIC DATA
FileStore* d_store_p;

mqbu::StorageKey d_dataFileKey;

MappedFileDescriptor d_dataFile;

MappedFileDescriptor d_journalFile;

MappedFileDescriptor d_qlistFile;
// PUBLIC TYPES

bsls::Types::Uint64 d_dataFilePosition;
/// Per-file-type information aggregated into a single struct.
struct FileInfo {
MappedFileDescriptor d_file;
bsl::string d_fileName;
bsls::Types::Uint64 d_filePosition;
bsls::Types::Uint64 d_outstandingBytes;

bsls::Types::Uint64 d_journalFilePosition;
// TRAITS
BSLMF_NESTED_TRAIT_DECLARATION(FileInfo, bslma::UsesBslmaAllocator)

bsls::Types::Uint64 d_qlistFilePosition;
// CREATORS
explicit FileInfo(bslma::Allocator* allocator);

bsl::string d_dataFileName;
private:
// NOT IMPLEMENTED
FileInfo(const FileInfo&) BSLS_KEYWORD_DELETED;
FileInfo& operator=(const FileInfo&) BSLS_KEYWORD_DELETED;
};

@678098 678098 Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This struct is the main change of this refactoring.
Do you think it's worth it? @kaikulimu


bsl::string d_journalFileName;
// PUBLIC DATA
FileStore* d_store_p;

bsl::string d_qlistFileName;
mqbu::StorageKey d_dataFileKey;

bsls::Types::Uint64 d_outstandingBytesJournal;
FileInfo d_data;

bsls::Types::Uint64 d_outstandingBytesData;
FileInfo d_journal;

bsls::Types::Uint64 d_outstandingBytesQlist;
FileInfo d_qlist;

bool d_journalFileAvailable;

Expand All @@ -105,8 +108,8 @@ struct FileSet BSLS_CPP11_FINAL {

private:
// NOT IMPLEMENTED
FileSet(const FileSet&) BSLS_CPP11_DELETED;
FileSet& operator=(const FileSet&) BSLS_CPP11_DELETED;
FileSet(const FileSet&) BSLS_KEYWORD_DELETED;
FileSet& operator=(const FileSet&) BSLS_KEYWORD_DELETED;

public:
// TRAITS
Expand All @@ -130,21 +133,20 @@ struct FileSet BSLS_CPP11_FINAL {
// -------------

// CREATORS
inline FileSet::FileInfo::FileInfo(bslma::Allocator* allocator)
: d_file()
, d_fileName(allocator)
, d_filePosition(0)
, d_outstandingBytes(0)
{
}

inline FileSet::FileSet(FileStore* store, bslma::Allocator* allocator)
: d_store_p(store)
, d_dataFileKey()
, d_dataFile()
, d_journalFile()
, d_qlistFile()
, d_dataFilePosition(0)
, d_journalFilePosition(0)
, d_qlistFilePosition(0)
, d_dataFileName(allocator)
, d_journalFileName(allocator)
, d_qlistFileName(allocator)
, d_outstandingBytesJournal(0)
, d_outstandingBytesData(0)
, d_outstandingBytesQlist(0)
, d_data(allocator)
, d_journal(allocator)
, d_qlist(allocator)
, d_journalFileAvailable(true)
, d_fileSetRolloverPolicyAlarm(false)
, d_inlineGc(false)
Expand Down
24 changes: 12 additions & 12 deletions src/groups/mqb/mqbs/mqbs_fileset.t.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,18 @@ static void test1_breathingTest()

BMQTST_ASSERT_EQ(obj.d_store_p, static_cast<mqbs::FileStore*>(0));
BMQTST_ASSERT_EQ(obj.d_dataFileKey.isNull(), true);
BMQTST_ASSERT_EQ(obj.d_dataFile.isValid(), false);
BMQTST_ASSERT_EQ(obj.d_journalFile.isValid(), false);
BMQTST_ASSERT_EQ(obj.d_qlistFile.isValid(), false);
BMQTST_ASSERT_EQ(obj.d_dataFilePosition, 0ULL);
BMQTST_ASSERT_EQ(obj.d_journalFilePosition, 0ULL);
BMQTST_ASSERT_EQ(obj.d_qlistFilePosition, 0ULL);
BMQTST_ASSERT_EQ(obj.d_dataFileName.empty(), true);
BMQTST_ASSERT_EQ(obj.d_journalFileName.empty(), true);
BMQTST_ASSERT_EQ(obj.d_qlistFileName.empty(), true);
BMQTST_ASSERT_EQ(obj.d_outstandingBytesJournal, 0ULL);
BMQTST_ASSERT_EQ(obj.d_outstandingBytesData, 0ULL);
BMQTST_ASSERT_EQ(obj.d_outstandingBytesQlist, 0ULL);
BMQTST_ASSERT_EQ(obj.d_data.d_file.isValid(), false);
BMQTST_ASSERT_EQ(obj.d_journal.d_file.isValid(), false);
BMQTST_ASSERT_EQ(obj.d_qlist.d_file.isValid(), false);
BMQTST_ASSERT_EQ(obj.d_data.d_filePosition, 0ULL);
BMQTST_ASSERT_EQ(obj.d_journal.d_filePosition, 0ULL);
BMQTST_ASSERT_EQ(obj.d_qlist.d_filePosition, 0ULL);
BMQTST_ASSERT_EQ(obj.d_data.d_fileName.empty(), true);
BMQTST_ASSERT_EQ(obj.d_journal.d_fileName.empty(), true);
BMQTST_ASSERT_EQ(obj.d_qlist.d_fileName.empty(), true);
BMQTST_ASSERT_EQ(obj.d_journal.d_outstandingBytes, 0ULL);
BMQTST_ASSERT_EQ(obj.d_data.d_outstandingBytes, 0ULL);
BMQTST_ASSERT_EQ(obj.d_qlist.d_outstandingBytes, 0ULL);
BMQTST_ASSERT_EQ(obj.d_journalFileAvailable, true);
BMQTST_ASSERT_EQ(obj.d_fileSetRolloverPolicyAlarm, false);
BMQTST_ASSERT(!obj.d_aliasedChunk_sp);
Expand Down
Loading
Loading