Add monitoring and automate add workload id for flashtestations #1
Conversation
avalonche
force-pushed
the
add-flashtestations-monitoring
branch
4 times, most recently
from
1c8755a to
4208907
Compare
avalonche
force-pushed
the
add-flashtestations-monitoring
branch
from
4208907 to
7e169ef
Compare
avalonche
force-pushed
the
add-flashtestations-monitoring
branch
from
f221a9c to
00061b9
Compare
| zap.Error(err), | ||
| zap.String("tx", txHash.Hex()), | ||
| ) | ||
| l2.registrationsError++ |
There was a problem hiding this comment.
issue:
handleRegistrationTx is ran in its own goroutine, potentially multiple at the same time (e.g. if chain-monitor is catching up after being down for a few seconds). meaning that if it changes state, then it must be protected somehow (e.g. behind a mutex).
the processBlock that calls handleRegistrationTx doesn't need mutexes b/c it's guaranteed to always have only 1 instance of it running (it's being triggered by the timer, which doesn't fire the next tick if the previous didn't finish).
There was a problem hiding this comment.
is it fine to have handleRegistrationTx blocking then? I don't think chain-monitor is latency sensitive as long as it can alert us within 5 - 10 minutes. Its also not expected to be called often
There was a problem hiding this comment.
what do you mean with "blocking"?
it's good that it's branched-off into its own gorouting b/c that makes main timer-loop small and tidy. otherwise, if it gets bloated with many RPC calls, and error handling of these calls, then it can happen that it will begin to fall back on tracking the latest block just because of that (already did happen).
so, ideally there should be something that would guard against concurrent access to the state (mutes, atomic) + retry logic (truncated exponential backoff)
3 participants
No description provided.