Host: only evict HA sequencer enclaves #2222
Conversation
| @@ -275,9 +275,8 @@ func (g *Guardian) mainLoop() { | |||
| g.logger.Trace("mainLoop - enclave status", "status", g.state.GetStatus()) | |||
| switch g.state.GetStatus() { | |||
| case Disconnected, Unavailable: | |||
| if unavailableCounter > 3 { | |||
| if unavailableCounter > 10 { | |||
There was a problem hiding this comment.
we might want to stick this in the host config in the event enclaves are getting stuck for a while (networking issues or something). when we're on K8s will be really easy to tweak
There was a problem hiding this comment.
Yeah agree but still a bit finger in the air on all this stuff (like it might end up being that we want to evict based on a timeout or just on failed batch submission). I'll add a todo to be configurable once we're happy with the trigger.
| e.logger.Info("not running in HA mode, no need to evict enclave", log.EnclaveIDKey, enclaveID) | ||
| return | ||
| } | ||
| if *e.activeSequencerID != *enclaveID { |
There was a problem hiding this comment.
any reason we can't compare the actual values here instead?
There was a problem hiding this comment.
I think this does compare the values (although I haven't fully internalised the quirks around this tbh lol). I had it as e.activeSequencerID == enclaveID below and Tudor fixed it, think it was comparing the pointers with that.
Why this change is needed
Treating single-enclave nodes and backup enclaves as being critical made some things more fragile than they needed to be.
Also evicting enclaves was happening a bit too quickly, causing race conditions at startup etc.
What changes were made as part of this PR
Slow down eviction. Enclave service only evicts active sequencers (no need for single-enclave validators).
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks