Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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[MQB, MWC]: report the number of tcp connections #384
Feat[MQB, MWC]: report the number of tcp connections #384
Changes from 7 commits
2ceb0f5
1a98e23
29506cc
d2171c8
3d29a39
3555a56
4cf0b83
0565d51
2e5e9e8
eff0b97
3c3025e
7f29d3a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we convert
bslma::ManagedPtr
tobsl::shared_ptr
. How does the ownership transfer works in this situiation? Is this operation safe? Are we sure that the underlyingStatContext
is not accidentally destroyed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this is a common practice:
blazingmq/src/groups/mwc/mwcio/mwcio_statchannelfactory.cpp
Lines 126 to 127 in 5aaf596
blazingmq/src/groups/mqb/mqbstat/mqbstat_clusterstats.cpp
Lines 254 to 258 in 5aaf596
blazingmq/src/groups/mqb/mqbblp/mqbblp_cluster.cpp
Lines 2582 to 2584 in 5aaf596
and so on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do per-channel stat contexts, then the number of connections will always be
0
or1
, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, I utilize the aggregation functionality Of StatContextTable here in order to deal only with the Channel's StatContext here. Otherways I would have to deal with its parent (port's
StatContext
), and, hence, store its shared_ptr here inStatChannel
classThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is useful, but also it's an interface change in
MWC
. Do we want to keep it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to check it in
portsDeleter()
as before the first snapshotnumSubcontexts()
returns zero, and I don't want to delete newStatContext
s in this case. It happens becauseStatContext::addSubcontext()
puts the newly created contexts tod_newSubcontexts
vector.blazingmq/src/groups/mwc/mwcst/mwcst_statcontext.cpp
Line 631 in 7d4b3d7
Then it moves them to
d_statContext
vector on nextStatContext::snapshot()
blazingmq/src/groups/mwc/mwcst/mwcst_statcontext.cpp
Line 339 in 7d4b3d7
However
StatContext::numSubcontexts()
doesn't take into accountd_newSubcontext
and will return 0 if there haven't been any stapshots yet.blazingmq/src/groups/mwc/mwcst/mwcst_statcontext.h
Lines 1136 to 1140 in 7d4b3d7
So if we don't check the number of snapshots in
portsDeleter()
we can unintentionally delete theStatContext
that has just been added.