Refactor metrics to simplify updating the values #187
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use a private data structure to track per-network metrics, then use that data structure as data source for the metrics callbacks. The once-added callbacks can then serve the metrics for registered definitions. In addition to that, when a definition ends, we deregister it from the per- network metrics data structure, so we stop serving metrics about it. We are no longer using Tokio mutexes since this would require async closures for the callbacks, and async closures are unstable. std::sync::Mutex works perfectly OK for this use and, in fact, Tokio documentation states that standard mutexes can work fine with Tokio code, and should be preferred in most cases. https://docs.rs/tokio/1.24.2/tokio/sync/struct.Mutex.html#which-kind-of-mutex-should-you-use indicates we should use the standard mutex when we are not going to hold the lock across await points. Since that is exactly what we are doing (there are no awaits involved in the set() and get() code for the data structure being locked), then we can simply use the simple blocking mutex, which is what we have done here.
DFINITYManu
force-pushed
the
refactormetrics
branch
from
ee1af94
to
7d30a57
Compare
DFINITYManu
force-pushed
the
refactormetrics
branch
from
7d30a57
to
41e6424
Compare
sasa-tomic
approved these changes
Feb 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Use a private data structure to track per-network metrics, then use that data structure as data source for the metrics callbacks. The once-added callbacks can then serve the metrics for registered definitions.
In addition to that, when a definition ends, we deregister it from the per- network metrics data structure, so we stop serving metrics about it.
We are no longer using Tokio mutexes since this would require async closures for the callbacks, and async closures are unstable. std::sync::Mutex works perfectly OK for this use and, in fact, Tokio documentation states that standard mutexes can work fine with Tokio code, and should be preferred in most cases.
https://docs.rs/tokio/1.24.2/tokio/sync/struct.Mutex.html#which-kind-of-mutex-should-you-use indicates we should use the standard mutex when we are not going to hold the lock across await points. Since that is exactly what we are doing (there are no awaits involved in the set() and get() code for the data structure being locked), then we can simply use the simple blocking mutex, which is what we have done here.