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

Fix[MQB]: respect protocol file size limits #539

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

678098
Copy link
Collaborator

@678098 678098 commented Dec 6, 2024

Before this PR, it was possible to set a file size not complaint to the protocol limitations. In this case, the cluster will work until it gets the first offset overflow, and then crash.

This PR:

  • Adds precise pre-calculated constants for DATA, JOURNAL, QLIST file sizes hard limits.
  • Adds the 1st line of checks in mqbblp::StorageManager::start (for non-FSM) and mqbc::StorageManager::start (for FSM) to gracefully return a failure RC on file size overflow.
  • Adds the 2nd line of checks (asserts in FileStoreSet::setDataFileSize etc), they should be never triggered in a normal workflow but left here just to be sure.
  • Improves consistency of RCs returned from [mqbblp/mqbc]::StorageManager::start
  • Adds UT to check file size hard limits for mqbc::StorageManager (there are no UTs for mqbblp::StorageManager).

The error logs look like this:

06DEC2024_00:29:20.649 25904:8394919744 ERROR /blazingmq/src/groups/mqb/mqbc/mqbc_storagemanager.cpp:3485 MQBC.STORAGEMANAGER Configured maxDataFileSize (34359738361) exceeds the protocol limit (34359738360) 

06DEC2024_00:29:20.649 25904:8394919744 ERROR /blazingmq/src/groups/mqb/mqbc/mqbc_storagemanager.cpp:3495 MQBC.STORAGEMANAGER Configured maxJournalFileSize (17179869181) exceeds the protocol limit (17179869180) 

06DEC2024_00:29:20.649 25904:8394919744 ERROR /blazingmq/src/groups/mqb/mqbc/mqbc_storagemanager.cpp:3505 MQBC.STORAGEMANAGER Configured maxQlistFileSize (17179869181) exceeds the protocol limit (17179869180) 

06DEC2024_00:29:20.649 25904:8394919744 ERROR /blazingmq/src/groups/mqb/mqbc/mqbc_storagemanager.cpp:3485 MQBC.STORAGEMANAGER Configured maxDataFileSize (8796093020160) exceeds the protocol limit (34359738360) 

@678098 678098 requested a review from a team as a code owner December 6, 2024 00:43
@678098 678098 requested review from kaikulimu and chrisbeard and removed request for a team December 6, 2024 00:43
Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 400 of commit 7fe8a8d has completed with FAILURE

Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 402 of commit f6a176c has completed with FAILURE

Copy link
Collaborator

@kaikulimu kaikulimu left a comment

Choose a reason for hiding this comment

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

One minor comment

src/groups/mqb/mqbc/mqbc_storagemanager.t.cpp Outdated Show resolved Hide resolved
@kaikulimu kaikulimu assigned 678098 and unassigned kaikulimu Dec 11, 2024
Signed-off-by: Evgeny Malygin <[email protected]>
Signed-off-by: Evgeny Malygin <[email protected]>
Signed-off-by: Evgeny Malygin <[email protected]>
@678098 678098 force-pushed the t2566_respect_file_size_limits branch from f6a176c to a169001 Compare December 11, 2024 09:03
@678098 678098 requested a review from kaikulimu December 11, 2024 09:03
@678098 678098 assigned kaikulimu and unassigned 678098 Dec 11, 2024
Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 410 of commit a169001 has completed with FAILURE

@678098 678098 merged commit 8625cab into bloomberg:main Dec 11, 2024
35 checks passed
@678098 678098 deleted the t2566_respect_file_size_limits branch December 11, 2024 09:57
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