-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not stop collecting events when journal entries change #9994
Do not stop collecting events when journal entries change #9994
Conversation
Failing tests are unrelated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we need a seccomp policy for ARM as well?
journalbeat/reader/journal.go
Outdated
case <-r.done: | ||
return | ||
default: | ||
changesChan <- c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like waitForChange
deadlocking -> go-routine/mem leak if waitUntilNewEventOrError
returns after waitForChange
is started but returns before it reads from changesChan. Due to the Wait
on shutdown of waitUntilNewEventOrError
, this function might be deadlocked as well.
Failing tests are unrelated on Travis. |
Hi! I'm trying to use the changes of this PR and I'm getting a massive wall of messages such as:
I'm building the journalbeat in an ubuntu container, and deploy it in an ubuntu container. Any suggestion as to what I might be doing incorrectly? My initial suspicion was that I'm not building it correctly but I am able to build v6.5.4. |
@amencarini The errors show up because Journalbeat tries to run syscalls which are not listed in the seccomp policy. As a workaround, you could set We need to fix this problem before releasing the new official Journalbeat image. |
@kvch thanks for your reply! Let me know if there is there anything I can do to help on this. |
Me again! I tried to put a 10s sleep before
So my impression is that whatever is involved in the On the other hand, forcing the rotation of the logs via |
Were you able to make it work using a seccomp profile? |
I haven't tried but I'm running this off Kubernetes and I can't find the equivalent option of |
|
||
r.backoff.Reset() | ||
return event, nil | ||
c := r.journal.Wait(100 * time.Millisecond) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: It's not too bad to wait for a few 100 millis, and I don't think we should change it here. But I'd prefer if we wouldn't have to wait. The Wait call uses sd_journal_wait, which itself uses ppoll. One should not close a file descriptor a poll operation is active on, but an alternate wait-without-wakeup-schema could be created by wasting a second polling file descriptor:
- create close-signaling file descriptor (e.g. create pipe, wastes 2 FDs)
- get journald fd via sd_journal_get_fd (it's the same journald polls on)
- poll on close-fd + journal-fd
- for handling async shutdown trigger close-fd (e.g. send some bytes)
- check who did send poll signal and handle accordingly
Requested changes have been applied. We should test if PR really helps with collector stopping and in which cases it might still fail.
129f86c
to
cea4c4b
Compare
I have just tested the seccomp policy for ARM in docker image |
Previously sd_journal_wait was not used. From now on all changes to journals are detected. I also added custom seccomp policy to Journalbeat. Closes elastic#9533 (cherry picked from commit cead4b6)
Previously sd_journal_wait was not used. From now on all changes to journals are detected. I also added custom seccomp policy to Journalbeat. Closes elastic#9533
…ournal entries change (elastic#10485) Cherry-pick of PR elastic#9994 to 6.x branch. Original message: Previously sd_journal_wait was not used. From now on all changes to journals are detected. I also added custom seccomp policy to Journalbeat. Closes elastic#9533
Previously
sd_journal_wait
was not used. From now on all changes to journals are detected.I also added custom seccomp policy to Journalbeat.
Closes #9533