Duplication of opmon "info" struct counter variables

[philiprodrigues]

(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 (eg mymoduleinfo::Info) with a std::atomic member variable of MyModule, and then copy the value of each atomic into the Info object inside get_info(). We
could avoid this duplication by holding a mymodule::Info as a member variable of MyModule and updating the counters in that object. Then the implementation of get_info() is just ci.add(m_info);. This approach would require the counters in the Info object to be atomic. Branch philiprodrigues/atomic-info-struct of appfwk has a proof-of-concept for one way to achieve this with moo templates.

[alessandrothea]

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 atomics 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.

[alessandrothea]

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.

[glehmannmiotto]

@alessandrothea , @philiprodrigues can this issue be followed up and eventually closed?

[alessandrothea]

I think so