Feat[bmqstoragetool]: PrintManager #529
Conversation
src/applications/bmqstoragetool/m_bmqstoragetool_recordprinter.h
Outdated
Show resolved
Hide resolved
waldgange
force-pushed
the
grepfriend
branch
5 times, most recently
from
de766a9 to
00c89fe
Compare
| @@ -44,8 +44,10 @@ Filters::Filters(const bsl::vector<bsl::string>& queueKeys, | |||
| mqbu::StorageKey key; | |||
| bsl::vector<bsl::string>::const_iterator uriIt = queueUris.cbegin(); | |||
| for (; uriIt != queueUris.cend(); ++uriIt) { | |||
| if (queueMap.findKeyByUri(&key, *uriIt)) { | |||
| d_queueKeys.insert(key); | |||
| bsl::optional<mqbu::StorageKey> key = queueMap.findKeyByUri( | |||
There was a problem hiding this comment.
Do we need optional here? Parameters::validateQueueNames guaranties that queue name is present in queueMap.
There was a problem hiding this comment.
Are you suggesting to change the code to something like this:
d_queueKeys.insert(queueMap.findKeyByUri(&key, *uriIt).value()) ?
Without checking if the optional really has the value? I agree that all the URIs are meant to be valid at this point. But are we winning anything with this potentially dangerous code? A temporary optional inside the function will be constructed anyway. But RVO will prevent it from extra copying to outside the function. The second copying will happen when we insert the value. The only way to avoid it is to return a const reference (which is not convinient, as we can't check if it's valid or dangling without an extra bool variable) or a pointer (which is not safe to return) to the internal value of the queueMap. Also mqbu::StorageKey is a very lightweight object. sizeof(mqbu::StorageKey) is only 5 bytes, sizeof(bsl::optional<mqbu::StorageKey>) is 6 bytes which is less than even a pointer. So it's not worth optimizing anythng here.
| @@ -76,6 +76,9 @@ int moveToLowerBound(mqbs::JournalFileIterator* jit, | |||
| } | |||
| left = jit->recordIndex(); | |||
| } | |||
| static int i = 0; | |||
| bsl::cout << (++i) << " advance(" << bsl::max((right - left) / 2, 1ULL) | |||
There was a problem hiding this comment.
Yes. I removed this
| @@ -2068,6 +2356,12 @@ static void test24_summaryWithQueueDetailsTest() | |||
| bmqtst::TestHelper::printTestName( | |||
| "OUTPUT SUMMARY WITH QUEUE DETAILS TEST"); | |||
|
|
|||
| bmqtst::TestHelperUtil::ignoreCheckDefAlloc() = true; | |||
| // Disable default allocator check for this test because | |||
| // EXPECT_CALL(PrinterMock::printRecordSummary(bsls::Types::Uint64, | |||
There was a problem hiding this comment.
No. It's a useful comment. It describes what causes the default allocator use.
|
|
||
| PRINTER_TYPE1 printer(ostream, &fields); | ||
| { | ||
| bsl::ostringstream s; |
There was a problem hiding this comment.
| bsl::ostringstream s; | |
| bsl::ostringstream s(allocator); |
| @@ -17,7 +17,7 @@ | |||
| #include <mqbs_filestoreprotocolprinter.h> | |||
|
|
|||
| // MQB | |||
| #include <mqbs_filestoreprotocolutil.h> | |||
| // #include <mqbs_filestoreprotocolutil.h> | |||
There was a problem hiding this comment.
Please remove if this include file not needed
Signed-off-by: Anton Pryakhin <[email protected]>
Signed-off-by: Anton Pryakhin <[email protected]>
![bmq-oss-ci[bot]](https://avatars.githubusercontent.com/u/1416818?s=60&v=4){kind=small}
In order to make the output of the storagetool more grep-friendly or/and to be able to parse it the following changes have been done:
PrintManagerPrintManagerPrintManagerhave been added:JsonLine(each Record is printed as a single-line json) andJsonPretty(each Record is printed as a multi-line json).