Adding Health endpoint #836
Conversation
health/health.go
Outdated
| h.lock.Lock() | ||
| defer h.lock.Unlock() | ||
|
|
||
| h.chainSynced = true |
There was a problem hiding this comment.
What would happen if, once the node is synched, for some reason, it gets out of synch for a period of time and need to re-sync? Would the health status still signaling the node is in sync?
There was a problem hiding this comment.
Good point, I've modified the ChainSync to ChainSyncStatus and marked as unhealthy if it's syncing.
There is a concern that this may make the ChainSync Status to flicker from Sync to UnSync a bit too often.
| } | ||
|
|
||
| // todo review time slots | ||
| healthy := time.Since(h.newBestBlock) >= 10*time.Second && |
There was a problem hiding this comment.
Is the first condition correct? Why are we considering healthy a node whose latest best block was seen more then 10 seconds ago?
There was a problem hiding this comment.
Yeah you're right, that's why the 10sec is marked as a todo, what would make sense for a this time gap you think ?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #836 +/- ##
==========================================
- Coverage 60.52% 60.38% -0.15%
==========================================
Files 213 215 +2
Lines 22931 23000 +69
==========================================
+ Hits 13879 13888 +9
- Misses 7916 7976 +60
Partials 1136 1136 ☔ View full report in Codecov by Sentry. |
|
do we have any tests for this new endpoint ? |
Description
A health check service that lives inside the node as an API endpoint.
I think it's a good illustrative start for a PR, there are a few tech choices made that I consider would be worth discussing, such as:
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist: