Skip to content
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

wilcard-file-reader: Fix detection of file delete/move when using ivykis poll events #4998

Conversation

HofiOne
Copy link
Collaborator

@HofiOne HofiOne commented Jun 13, 2024

This PR has the following changes:

  1. Fixes a crash in log pipe queue during file deletion and EOF detection

    The crash was caused by a concurrency issue in the EOF and file deletion detection when using a wildcard-file() source.

    If a file is written after being deleted (e.g. with an application keeping the file open), or if these events happen concurrently, the file state change poller mechanism might schedule another read cycle even though the file has already been marked as fully read and deleted.

    To prevent this re-scheduling between these two checks, the following changes have been made:
    Instead of maintaining an internal EOF state in the WildcardFileReader, when a file deletion notification is received, the poller will be signaled to stop after reaching the next EOF. Only after both conditions are set the reader instance will be deleted.

    Fixes: syslog-ng 3.24.1 sometimes crash in log_pipe_queue #4989

    The fix is inspired by @MrAnno's #160

  2. Fixed the file deletion and removal detection when the file-reader uses poll_fd_events to follow file changes, which were mishandled. For example, files that were moved or deleted (such as those rolled by a log-rotator) were read to the end but never read again if they were not touched anymore, therefore switching to the new file never happened.

  3. Added a dedicated monitor_freq option to control the poll frequency of the change detection in the directories separately when the poll method is selected via the monitor-method() option.

    The monitor-method() option controls only the change detection method in the directories, not the following of the file changes, and if poll is the selected method the frequency must not necessarily be the same, e.g. if the (earlier) commonly used follow-freq() is set to 0 for switching to the poll_fd_events method for file content change detection, that also might be meant a directory change poll with zero delays (if monitor-method() was set to poll as well), and that could cause a heavy CPU load unnecessarily.

Documented in syslog-ng/syslog-ng.github.io#101

Signed-off-by: Hofi [email protected]

@HofiOne HofiOne marked this pull request as draft June 13, 2024 12:05
@kira-syslogng
Copy link
Contributor

Build FAILURE

@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from 07238c9 to 41bfd3a Compare June 13, 2024 12:24
HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Jun 13, 2024
@kira-syslogng
Copy link
Contributor

Build FAILURE

HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Jun 17, 2024
@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from 9f066f8 to 2ba0367 Compare June 17, 2024 11:20
@kira-syslogng
Copy link
Contributor

Build FAILURE

HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Jun 17, 2024
@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from 2ba0367 to 70942b0 Compare June 17, 2024 12:57
@kira-syslogng
Copy link
Contributor

Build FAILURE

HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Jun 18, 2024
@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from 70942b0 to fc67fe9 Compare June 18, 2024 08:45
@kira-syslogng
Copy link
Contributor

Build FAILURE

HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Jun 22, 2024
@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from fc67fe9 to 1d92aed Compare June 24, 2024 07:52
@HofiOne HofiOne changed the title wilcard-file-reader: Fix crash in log pipe queue during file deletion and EOF detection wilcard-file-reader: Fix detection of file delete/move/add when using ivykis pool events Jun 24, 2024
@kira-syslogng
Copy link
Contributor

Build FAILURE

HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Jun 24, 2024
@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch 2 times, most recently from ed5699a to 0a7e0b1 Compare June 24, 2024 09:51
@kira-syslogng
Copy link
Contributor

Build FAILURE

HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Jun 24, 2024
@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from f8a2eee to 298063b Compare June 24, 2024 16:05
@kira-syslogng
Copy link
Contributor

Build FAILURE

@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch 2 times, most recently from 4ca3b23 to c1d4e4b Compare June 24, 2024 17:01
@kira-syslogng
Copy link
Contributor

Build FAILURE

Also, added a one-time work trigger function if a work is not in progress already
Checking the reader openness is public now.
Signed-off-by: Hofi <[email protected]>
Update watches reschedule the io handlers only if the new poll checker resulting to true
Signed-off-by: Hofi <[email protected]>
… on file deleted exit flow for all the poll readers

Signed-off-by: Hofi <[email protected]>
…tect reader liveness and forwarding now all the pipe notifications

Signed-off-by: Hofi <[email protected]>
Also, added the poll events fn getter
Signed-off-by: Hofi <[email protected]>
…to create collection entry hashes to detect file renaming as well

Signed-off-by: Hofi <[email protected]>
… (preparation of test modifications)

Signed-off-by: Hofi <[email protected]>
…aders

Do not use a direct file reload on filemoved detected in the ivikys file poll, it can lead to multiple processing of the same file.
Directory monitor can notify now correctly about a file move, use the same notification and processing flow in every case.
Signed-off-by: Hofi <[email protected]>
@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from b984dfa to 9e32406 Compare July 15, 2024 16:06
@kira-syslogng
Copy link
Contributor

Build FAILURE

@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch 2 times, most recently from fe88ef4 to 9cedf9a Compare July 16, 2024 11:00
@kira-syslogng
Copy link
Contributor

Build FAILURE

@HofiOne
Copy link
Collaborator Author

HofiOne commented Jul 16, 2024

@kira-syslogng test this please;

@HofiOne HofiOne force-pushed the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch from 9cedf9a to d20281f Compare July 16, 2024 12:14
@HofiOne HofiOne merged commit 6e4fad8 into syslog-ng:master Jul 16, 2024
23 checks passed
@HofiOne HofiOne deleted the syslog-ng-3241-sometimes-crash-in-log_pipe_queue-4989 branch July 16, 2024 14:49
HofiOne added a commit to syslog-ng/syslog-ng.github.io that referenced this pull request Jul 16, 2024
Added the new monitor-freq() option description

Documentation of syslog-ng/syslog-ng#4998

Signed-off-by: Hofi [[email protected]](mailto:[email protected])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

syslog-ng 3.24.1 sometimes crash in log_pipe_queue
4 participants