Add goroutine to check if Asterisk entity ID has changed.#33
Add goroutine to check if Asterisk entity ID has changed.#33Ulexus merged 2 commits intoCyCoreSystems:masterfrom
Conversation
Ulexus
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mtryfoss)
a discussion (no related file):
I agree; I think a standard ticker in a goroutine is a better choice. Much lower load when it matters.
server/server.go, line 255 at r1 (raw file):
continue } if s.AsteriskID != info.SystemInfo.EntityID {
I would additionally filter out the case where s.AsteriskID == "", to ignore startup race conditions. The 10s delay is probably sufficient, but it is implicit rather than explicit.
if s.AsteriskID == "" {
continue
}
mtryfoss
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @Ulexus)
a discussion (no related file):
Previously, Ulexus (Seán C. McCord) wrote…
I agree; I think a standard ticker in a goroutine is a better choice. Much lower load when it matters.
Added comment in types.go
server/server.go, line 255 at r1 (raw file):
Previously, Ulexus (Seán C. McCord) wrote…
I would additionally filter out the case where
s.AsteriskID == "", to ignore startup race conditions. The 10s delay is probably sufficient, but it is implicit rather than explicit.if s.AsteriskID == "" { continue }
I don't think that is necessary? On startup you already got this on line 128:
if s.AsteriskID == "" {
return errors.New("empty Asterisk ID")
}
In my head, s.AsteriskID can never be blank after that.
Ulexus
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @Ulexus)
server/server.go, line 255 at r1 (raw file):
Previously, mtryfoss wrote…
I don't think that is necessary? On startup you already got this on line 128:
if s.AsteriskID == "" {
return errors.New("empty Asterisk ID")
}In my head, s.AsteriskID can never be blank after that.
Yep; okay. I like explicit checks for things like that, but you are absolutely correct. There are too many places where the Asterisk ID is required before getting to this point. It would be pretty difficult to orphan this.
Ulexus
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @Ulexus)
Ulexus
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
New approach for same issue, but now simple a simple os.Exit(1). Changed to goroutine since we don't need access to waitgroup in listen() function.
Now the process can run for up to 10 seconds after an Asterisk restart before a check.
One could also decode StasisStart events in eventhandler and check against node id to exit on the very first call if that's before the ticker interval, but I don't think that's worth the effort of the all the extra decoding.
This change is