Open philiprodrigues opened 3 years ago
Hi, sorry for not having managed to answer before.
I think there are 2 different aspects to this issue. I agree that having to keep a copy of each variable as a data member is cumbersome/error prone/generally suboptimal, and that opmonlib
's Info
classes could be put to a better use.
For what thread safety is concerned, the use of atomic
s in data members is one option, suitable for cases where monitoring variables within the same info-block are not required to be 100% consistent. If consistency is needed, e.g. if multiple variables are sampled at the same point in the code, a mutex-based approach might be more suitable to ensure that there is no chance of mismatch. Obviously this level of consistency is not essential now, but it will become important for debugging when the system scales up.
If I recall correctly, this is one of the reasons why we moved away from the original monitoring model based on atomic
data members and a monitorable quantities registry. At the time we didn't make any provision for thread safety nor had a discussion about the problem in general but it looks like now it's the time for that discussion.
In terms of where the issue belongs, I'd say with opmonlib
.
Incidentally, a recent fix to moo
has finally allowed the introduction of custom opmonlib Info*
templates.
At the moment they are used to standardise the definition of the class_name
data member, but can be expanded along the lines of what you suggest once we have clarified what are the cases we want to support and with what patterns.
@alessandrothea , @philiprodrigues can this issue be followed up and eventually closed?
I think so
(Maybe this issue should go in a different package, but probably the solution should go here).
The opmon
get_info()
function is typically called from a different thread than the one in which the opmon counters are actually filled. That means we have to duplicate each of the members of the info struct (egmymoduleinfo::Info
) with astd::atomic
member variable ofMyModule
, and then copy the value of each atomic into theInfo
object insideget_info()
. We could avoid this duplication by holding amymodule::Info
as a member variable ofMyModule
and updating the counters in that object. Then the implementation ofget_info()
is justci.add(m_info);
. This approach would require the counters in theInfo
object to be atomic. Branchphiliprodrigues/atomic-info-struct
of appfwk has a proof-of-concept for one way to achieve this with moo templates.