Fix concurrency in ES client message buffer #54
Conversation
"fatal error: concurrent map writes" /go/src/github.com/infrawatch/sg-core/plugins/application/elasticsearch/main.go:95 /go/src/github.com/infrawatch/sg-core/pkg/bus.go:65 calls Application callbacks as a goroutine which results in concurrency in ReceiveEvent(). I've added a mutex to protect access.
|
I've tested this and it fixes the problem; I can put 100k logs through the system and it doesn't crash, whereas before the fix (and when my fix didn't include protection for the initialization on L95) it would reliably crash before the first 10k. I am planning to compare it vs. #53 (developed concurrently) to see if there are any performance differences. I would expect this one to be faster anyways, but it's worth a shot just to see. |
There was a problem hiding this comment.
I am curious to see which one runs faster, although I too suspect this one will because of fewer calls to Lock() and Unlock() methods. The golang recommendation is to lock data types rather than methods because it helps to keep future developers from introducing concurrency bugs, but if this is more performant it's definitely worth it
|
I'll be testing #53 and will prefer it if there is no obvious penalty.
Lol - except we already did that and the bug came anyways, right? |
|
No? Not sure where you mean |
I mean we already had a locked datatype at hand, but the concurrency bug appeared anyways since it wasn't used. I understand you meant something slightly different. In any case, I'm trying to pull together a lab to test #53 today. Hope to merge it by EOD |
|
Closed in favor of #53 |
"fatal error: concurrent map writes"
/go/src/github.com/infrawatch/sg-core/plugins/application/elasticsearch/main.go:95
/go/src/github.com/infrawatch/sg-core/pkg/bus.go:65 calls Application callbacks as a goroutine which results in concurrency in ReceiveEvent(). I've added a mutex to protect access.