Conversation
kalleep
left a comment
There was a problem hiding this comment.
Nice! Just left one comment
| c.mut.RLock() | ||
| var rcs []*relabel.Config | ||
| if len(c.args.RelabelRules) > 0 { | ||
| rcs = alloy_relabel.ComponentToPromRelabelConfigs(c.args.RelabelRules) | ||
| } | ||
|
|
||
| return nil | ||
| for _, l := range c.targets { | ||
| err := l.Stop() | ||
| if err != nil { | ||
| level.Error(c.opts.Logger).Log("msg", "error while stopping syslog listener", "err", err) | ||
| } | ||
| } | ||
| c.mut.RUnlock() | ||
|
|
||
| // Stop draining routine | ||
| cancel() | ||
|
|
||
| // Create new targets | ||
| c.mut.Lock() | ||
| defer c.mut.Unlock() |
There was a problem hiding this comment.
🤔 is there a potential race condition / panic here if a reload happens at the same time as shutdown? Shutdown is going to trigger
alloy/internal/component/loki/source/syslog/syslog.go
Lines 73 to 88 in c2f9bbb
I don't know that it's a realistic/possible scenario but I do wonder if it's safer to take a write lock before we attempt to mutate the state of the targets.
There was a problem hiding this comment.
Thanks for taking a closer look.
if a reload happens at the same time as shutdown
if I understood right and by "reload" you mean call to reloadTargets - then this is not possible to happen at the same time as shutdown deferred function, because reloadTargets happens on the same thread as the Run function uses.
Let me know if you were thinking of a different scenario
cf995f0 to
60dc7a8
Compare
PR Description
loki.source.syslog had a few race conditions:
Which issue(s) this PR fixes
Internal escalation https://github.com/grafana/support-escalations/issues/18202
Notes to the Reviewer
PR Checklist