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

Feat[Storagetool]: List queue appid numrecords #534

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

Conversation

bbpetukhov
Copy link
Contributor

@bbpetukhov bbpetukhov commented Nov 29, 2024

NB: This PR depends on #531 and will be ready for review as soon as #531 is merged

Describe your changes
Added new functional to bmqstoragetool to list number of all types of records per queue.
Supported resolution of Queue Key and App Key.
Added extra cmd line arg --min-records-per-queue <threshold> to restrict displaying queues with low number of records (less than given threshold)

Testing performed

  • Manual testing with a few real journal file and corresponding CSL files.
  • with/without CSL file to make sure that Queue Keys resolution working.
  • Added a new Unit Test to check storage tool output

Additional context

Usage example:
./bmqstoragetool.tsk --summary --min-records-per-queue 1012012 --journal-file file.bmq_journal --csl-file file.bmq_csl

Sample Output (section related to this PR):

...
Number of records per Queue:
    Queue                 : 1234567890
    Total Records         : 333334
    Num Queue Op Records  : 1
    Num Message Records   : 111111
    Num Confirm Records   : 111111
    Num Delete Records    : 111111
    Queue                 : 1234567890A
    Total Records         : 1111481
    Num Queue Op Records  : 881
    Num Message Records   : 100
    Num Confirm Records   : 670
    Num Records Per App   : 1234567890B=80 1234567890C=90 1234567890D=100 1234567890E=100 1234567890F=100 12345678910=100 12345678911=100 
    Num Delete Records    : 100

Sample Output showcasing the resolution of Queue Key and App Key:

Run without CSL file:

Number of records per Queue:
    Queue Key             : E12EF21974
    Total Records         : 5
    Num Queue Op Records  : 1
    Num Message Records   : 1
    Num Confirm Records   : 2
    Num Records Per App   : 793E7A6FD3=1 D6B7D942A8=1
    Num Delete Records    : 1

Run with CSL file (i.e. --csl-file ./bmq_csl_20240930_090323_CF83E8E13A.bmq_csl):

Number of records per Queue:
    Queue Key             : E12EF21974
    Queue URI             : bmq://bmq.test.persistent.fanout/dima
    Total Records         : 5
    Num Queue Op Records  : 1
    Num Message Records   : 1
    Num Confirm Records   : 2
    Num Records Per App   : baz=1 foo=1
    Num Delete Records    : 1

bbpetukhov and others added 12 commits November 28, 2024 12:17
Signed-off-by: Dmitrii Petukhov <[email protected]>
Signed-off-by: Dmitrii Petukhov <[email protected]>
Signed-off-by: Dmitrii Petukhov <[email protected]>
Signed-off-by: Dmitrii Petukhov <[email protected]>
Signed-off-by: Dmitrii Petukhov <[email protected]>
Signed-off-by: Dmitrii Petukhov <[email protected]>
Signed-off-by: Dmitrii Petukhov <[email protected]>
Signed-off-by: Dmitrii Petukhov <[email protected]>
@bbpetukhov bbpetukhov force-pushed the list-queue-appid-numrecords branch from 608ae0c to 185194d Compare November 29, 2024 18:16
"min number of records per queue for detailed info to be displayed",
balcl::TypeInfo(&arguments.d_minRecordsPerQueue),
balcl::OccurrenceInfo(
bsl::numeric_limits<bsls::Types::Int64>::max())},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider to use zero as default value. In most configs usually zero is used as default to turn off the feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did try to use Uint64, but balcl::TypeInfo does not support it. So I had to switch to Int64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we discussed in person I keep Int64 for cmd line argument and use Uint64 in Parameters class.

@bbpetukhov bbpetukhov changed the title List queue appid numrecords Feat[Storagetool]: List queue appid numrecords Dec 2, 2024
Signed-off-by: Dmitrii Petukhov <[email protected]>
@pniedzielski pniedzielski self-assigned this Dec 2, 2024
@bbpetukhov bbpetukhov force-pushed the list-queue-appid-numrecords branch from 1d1fc97 to 1ef7d9b Compare December 2, 2024 16:50
@bbpetukhov bbpetukhov force-pushed the list-queue-appid-numrecords branch from 9d70563 to ff069a3 Compare December 3, 2024 13:26
@bbpetukhov bbpetukhov marked this pull request as ready for review December 3, 2024 17:46
@bbpetukhov bbpetukhov requested a review from a team as a code owner December 3, 2024 17:46
@bbpetukhov bbpetukhov force-pushed the list-queue-appid-numrecords branch from dd1a0a7 to 313e91e Compare December 5, 2024 13:05
@bbpetukhov bbpetukhov force-pushed the list-queue-appid-numrecords branch from 74f5f11 to 988cccf Compare December 20, 2024 17:33
@bbpetukhov bbpetukhov force-pushed the list-queue-appid-numrecords branch from 3633b88 to c3a2fc8 Compare January 9, 2025 15:10
@bbpetukhov bbpetukhov force-pushed the list-queue-appid-numrecords branch from 5b31e2b to e28074b Compare January 13, 2025 09:40
@bbpetukhov
Copy link
Contributor Author

Merged with upstream main. Ready for review.

// Prepare expected output
bmqu::MemOutStream expectedStream(bmqtst::TestHelperUtil::allocator());
expectedStream << "5 message(s) found.\n";
bsl::vector<const char*> fields(bmqtst::TestHelperUtil::allocator());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not needed to fix now, but I opened an issue for this thing
#564

@@ -106,6 +106,9 @@ struct CommandLineArguments {
bool d_confirmed;
/// Show only messages, confirmed by some of the appId's
bool d_partiallyConfirmed;
// Show only messages, confirmed by some of the appId's
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like comment duplication here and below. Consider to use /// as for other cases.

@@ -204,6 +204,22 @@ void outputFooter(bsl::ostream& ostream,
// class SearchResult
// ==================

bool SearchResult::processQueueOpRecord(
Copy link
Collaborator

Choose a reason for hiding this comment

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

SearchResult is an interface and has no implementation. Why do we need dummy implementation for these methods?

@@ -192,7 +192,7 @@

#if BSLS_COMPILERFEATURES_SIMULATE_CPP11_FEATURES
// Include version that can be compiled with C++03
// Generated on Tue Oct 15 17:39:53 2024
// Generated on Thu Jan 9 16:44:32 2025
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems it is not needed to update these files.

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.

4 participants