Skip to content
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

Performance[BMQ, MQB, MWC]: performance tune-up [1] #375

Merged
merged 4 commits into from
Aug 8, 2024
Merged

Conversation

dorjesinpo
Copy link
Collaborator

First half of 181

  1. Increase Event (batch) size. Keep PUT soft limit
  2. Use bsls::BlockGrowth::BSLS_CONSTANT (instead of geometric) for pools
  3. Avoid mwcu::MemOutStream while validating in Cluster
  4. StorageManager::processReceiptEvent in IO thread (minus one thread switch)
  5. FileStore generates Receipt at the end of received batch processing

@dorjesinpo dorjesinpo added the enhancement New feature or request label Jul 25, 2024
@dorjesinpo dorjesinpo requested a review from a team as a code owner July 25, 2024 14:53
@dorjesinpo dorjesinpo force-pushed the dev/tune-up-#2 branch 2 times, most recently from b1e9d2d to 05c6345 Compare July 29, 2024 13:58
@dorjesinpo dorjesinpo force-pushed the dev/tune-up-#2 branch 2 times, most recently from ace1e86 to 4be456c Compare July 29, 2024 15:03
src/groups/bmq/bmqp/bmqp_eventutil.t.cpp Outdated Show resolved Hide resolved
src/groups/bmq/bmqp/bmqp_protocol.h Show resolved Hide resolved
src/groups/mqb/mqbblp/mqbblp_cluster.cpp Outdated Show resolved Hide resolved
src/groups/mqb/mqbblp/mqbblp_cluster.cpp Show resolved Hide resolved
src/groups/mqb/mqbblp/mqbblp_cluster.h Show resolved Hide resolved
src/groups/mqb/mqbblp/mqbblp_remotequeue.cpp Outdated Show resolved Hide resolved
Comment on lines 4848 to 4865
bool doRetry = false;
do {
buildRc = d_storageEventBuilder.packMessage(type,
d_config.partitionId(),
0, // flags
journalOffsetWords,
journalRecordBlobBuffer);
if (buildRc == bmqt::EventBuilderResult::e_EVENT_TOO_BIG && !doRetry) {
flushIfNeeded(true);

doRetry = true;
}
else {
doRetry = false;
}
} while (doRetry);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just an idea:

  • get rid of a loop
  • get rid of tmp bool doRetry
  • add performance hint
  • as a bonus, in a debugger you will see that you are currently on the first try or on the second
Suggested change
bool doRetry = false;
do {
buildRc = d_storageEventBuilder.packMessage(type,
d_config.partitionId(),
0, // flags
journalOffsetWords,
journalRecordBlobBuffer);
if (buildRc == bmqt::EventBuilderResult::e_EVENT_TOO_BIG && !doRetry) {
flushIfNeeded(true);
doRetry = true;
}
else {
doRetry = false;
}
} while (doRetry);
buildRc = d_storageEventBuilder.packMessage(type,
d_config.partitionId(),
0, // flags
journalOffsetWords,
journalRecordBlobBuffer);
if (BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY(buildRc == bmqt::EventBuilderResult::e_EVENT_TOO_BIG)) {
BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
flushIfNeeded(true);
buildRc = d_storageEventBuilder.packMessage(type,
d_config.partitionId(),
0, // flags
journalOffsetWords,
journalRecordBlobBuffer);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would prefer a version calling packMessage once, loop is ok, BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY is a good idea

src/groups/mqb/mqbs/mqbs_filestore.cpp Show resolved Hide resolved
const bmqp::StorageHeader& header = iter.header();
if (pid != header.partitionId()) {
// A storage event is sent by 'source' cluster node. The node may
// be primary for one or more partitions, but as per the BMQ
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// be primary for one or more partitions, but as per the BMQ
// be primary for one or more partitions, but as per the BlazingMQ

src/groups/mqb/mqbs/mqbs_filestore.h Show resolved Hide resolved
@678098 678098 changed the title performance tune-up #1 Feat[BMQ, MQB, MWC]: performance tune-up [1] Jul 29, 2024
678098
678098 previously approved these changes Jul 29, 2024
Copy link
Collaborator

@678098 678098 left a comment

Choose a reason for hiding this comment

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

One small comment

@@ -494,6 +523,8 @@ class Cluster : public mqbi::Cluster,
/// Execute `initiateShutdown` followed by `stop` and SIGINT
void terminate(mqbu::ExitCode::Enum reason);

static const char* validationResult(const ValidationResult& result);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This declaration doesn't have a definition now, right?

Comment on lines 526 to 527
static const char* validationResult(const ValidationResult& result);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
static const char* validationResult(const ValidationResult& result);

Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
@dorjesinpo dorjesinpo removed the request for review from a team August 8, 2024 17:09
@dorjesinpo dorjesinpo merged commit 7d4b3d7 into main Aug 8, 2024
31 checks passed
@678098 678098 deleted the dev/tune-up-#2 branch August 8, 2024 17:23
@678098 678098 changed the title Feat[BMQ, MQB, MWC]: performance tune-up [1] Performance[BMQ, MQB, MWC]: performance tune-up [1] Aug 29, 2024
alexander-e1off pushed a commit to alexander-e1off/blazingmq that referenced this pull request Oct 24, 2024
* performance tune-up bloomberg#1

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

* Reverting the change causing IT failure

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

* Addressing review

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

* cleaning

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

---------

Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
alexander-e1off pushed a commit to alexander-e1off/blazingmq that referenced this pull request Oct 24, 2024
* performance tune-up bloomberg#1

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

* Reverting the change causing IT failure

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

* Addressing review

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

* cleaning

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

---------

Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
alexander-e1off pushed a commit to alexander-e1off/blazingmq that referenced this pull request Oct 24, 2024
* performance tune-up bloomberg#1

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

* Reverting the change causing IT failure

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

* Addressing review

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

* cleaning

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

---------

Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants