-
Notifications
You must be signed in to change notification settings - Fork 245
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_: RPC providers detailed statuses #5923 #5924
base: develop
Are you sure you want to change the base?
Conversation
Jenkins BuildsClick to see older builds (72)
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## develop #5924 +/- ##
============================================
- Coverage 47.02% 10.54% -36.48%
============================================
Files 832 832
Lines 137725 137070 -655
============================================
- Hits 64763 14459 -50304
- Misses 65429 120751 +55322
+ Partials 7533 1860 -5673
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ac6650f
to
5164e3d
Compare
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.
Much cleaner approach than we had.
|
||
// Subscribe allows providers to receive notifications about changes. | ||
func (p *ProvidersHealthManager) Subscribe() chan struct{} { | ||
p.mu.Lock() |
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 p.mu
is used to guard p. aggregator
could we use another mutex to guard p. subscribers
only?
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 think it shouldn't be a problem to protect the whole struct state with the same mutext, unless there is frequent access to Update
and Subscribe
methods at the same time.
Ideally we should not use 2 mutexes in the same struct to avoid unintentional deadlocks. Otherwise we have to ensure consistent locking order. Or I can use a sync.Map
for subscribers
// Subscribe allows clients to receive notifications about changes. | ||
func (b *BlockchainHealthManager) Subscribe() chan struct{} { | ||
ch := make(chan struct{}, 1) | ||
b.mu.Lock() |
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.
The same here, does it make sense to have separate mutex for subscribers?
b0651df
to
cd640b3
Compare
cd640b3
to
eb10b20
Compare
PR Summary:
This PR introduces a new RPC provider health monitor, as discussed here. It's moved to a separate PR without changing existing logic (will be introduced as a separate PR)
please read before review
1st commit - healthmanager
provider_health_managers
s and sends updates.2nd commit - integration
provider_health_manager
blockchain_health_manager
and sends a new event to the wallet feed on every updateImportant changes:
Usage:
BlockchainHealthManager
withNewBlockchainHealthManager()
ProviderHealthManager
for each chain viaNewProvidersHealthManager(chainID)
ProvidersHealthManager
withBlockchainHealthManager::RegisterProvidersHealthManager
ProvidersHealthManager::Update()
BlockchainHealthManager::Subscribe()
and get status fromGetFullStatus()
BlockchainHealthManager::Stop()
Closes #5923