libbeat: Stop autodiscover runners in a goroutine #7170
Conversation
|
@jsoriano we should backport this to, at least, 6.3 |
jsoriano
added
needs_backport
|
In case it gives any clue, |
libbeat/autodiscover/autodiscover.go
Outdated
| @@ -178,7 +178,7 @@ func (a *Autodiscover) handleStop(event bus.Event) { | |||
|
|
|||
| if runner := a.runners.Get(hash); runner != nil { | |||
| logp.Info("Autodiscover stopping runner: %s", runner) | |||
| runner.Stop() | |||
| go runner.Stop() | |||
| a.runners.Remove(hash) | |||
There was a problem hiding this comment.
the hash should only be removed if stopping is finished I would think
There was a problem hiding this comment.
We actually want to remove it here, if a new event arrives to start the same module we want that to happen, even if the previous instance is still stopping
There was a problem hiding this comment.
I see, so adding and removing is synchronous. To make it more clear in the code we should add a comment here and move the stopping command after the removal of the hash.
Any concerns about leaking go routines here if Stop() gets stuck?
|
Could this cause some race conditions? |
|
@ruflin it is possible, yes, not sure what happens if a runner is stopped twice. |
|
I don't think it's possible, events are processed by autodiscover one bye one, it acts as a state machine. This should ensure that the event is removed from the list by the moment you process the next event. For instance 2 stop events (this can happen) would go like this:
|
jsoriano
force-pushed
the
autodiscover-async-stop
branch
from
10a77c1
to
26ef20f
Compare
|
Added changelog. |
|
Change LGTM but I think we should document what we discussed here in the code for the next person touching this code. |
Stopping runners can be a blocking operation, what can block the autodiscover event loop. Run it in a goroutine so events can continue being handled.
jsoriano
force-pushed
the
autodiscover-async-stop
branch
from
26ef20f
to
e5050e7
Compare
|
@ruflin ok, I have added a comment explaining the decissions there. |
|
jenkins, test this |
jsoriano
added
v6.3.0
and removed
needs_backport
Stopping runners can be a blocking operation, what can block the autodiscover event loop. Run it in a goroutine so events can continue being handled. (cherry picked from commit 500ab7c)
…stic#7186) Stopping runners can be a blocking operation, what can block the autodiscover event loop. Run it in a goroutine so events can continue being handled. (cherry picked from commit eb58c3a)
Stopping runners can be a blocking operation, what can block the
autodiscover event loop. Run it in a goroutine so events can continue
being handled.