storage: messing with Liveness.IsHealthy #43873
Conversation
NodeLiveness.IsHealthy() was passing a non-sensical threshold to the function determining whether a node is supposed to be dead. It was passing the duration of a liveness period, but that duration had already been incorporated in the liveness' expiration. I believe this is inconsequential because this IsHealthy() only cares about LIVE nodes, and that state was not affected by this threshold (i.e. if the node would have been considered dead according to any positive threshold, it won't be onsidered live). Release note: None
IsHealthy() is a very specific function with a single caller. It doesn't deserve to live next to the other liveness code (which code is confusing enough without it). This patch embeds it in the caller. Release note: None
|
This change is |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, 1 of 2 files at r2.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @andreimatei, and @asubiotto)
pkg/server/status.go, line 676 at r2 (raw file):
ls := l.LivenessStatus( clock.PhysicalTime(), 0, /* threshold */
Do you not want to remove this parameter while you're here?
There was a problem hiding this comment.
I'm cool with this under the expectation that you clean up this mess and remove that threshold in future PRs.
Reviewable status:
There was a problem hiding this comment.
yeah, I'm trying
Reviewable status:
There was a problem hiding this comment.
bors r+
Reviewable status:
Build failed |
There was a problem hiding this comment.
acceptance/version-upgrade timed out. Couldn't repro.
bors r+
Reviewable status:
43873: storage: messing with Liveness.IsHealthy r=andreimatei a=andreimatei See individual commits. Co-authored-by: Andrei Matei <[email protected]>
Build succeeded |
See individual commits.