Support logrotate on Windows correctly. Fix #2446 #2663
Conversation
In Windows multi-process environment, we cannot rename a file which is opened by another process. Then, we should do this: * Separate log file for each process on Windows Signed-off-by: Hiroshi Hatake <hatake@clear-code.com>
|
This patch doesn't change file name when no rotate option, right? |
|
Right. |
| def init(process_type, worker_id) | ||
| @opts[:process_type] = process_type | ||
| @opts[:worker_id] = worker_id | ||
|
|
||
| if @path && @path != "-" | ||
| @logdev = if @log_rotate_age || @log_rotate_size | ||
| Fluent::LogDeviceIO.new(@path, shift_age: @log_rotate_age, shift_size: @log_rotate_size) | ||
| Fluent::LogDeviceIO.new(Fluent.windows? ? | ||
| worker_id_suffixed_path(worker_id, @path) : @path, |
There was a problem hiding this comment.
if the process_type is :supervisor, this method call looks unnecessary for me. supervisor's worker id is 0 so the name of log file for supervisor becomes log-supervisor-0.log, right?
There was a problem hiding this comment.
if the
process_typeis:supervisor, this method call looks unnecessary for me.
Yeah, I think so too before testing for it.
If log-supervisor-0.log and log-0.log are not separated, Fluentd still complains the following error:
log shifting failed. Permission denied @ rb_file_s_rename - ([Some sort of moving files])
log writing failed. closed stream
log shifting failed. closed stream
log writing failed. closed stream
log shifting failed. closed stream
log writing failed. closed stream
log shifting failed. closed stream
log writing failed. closed stream
log shifting failed. closed stream
log writing failed. closed stream
log shifting failed. closed stream
log writing failed. closed stream
I have no idea for this issue....
supervisor's worker id is 0 so the name of log file for supervisor becomes log-supervisor-0.log, right?
Yes.
There was a problem hiding this comment.
If log-supervisor-0.log and log-0.log are not separated,
I meant it was better the file name of a supervisor was log-supervisor.log not log-0.log.
If you have the intention to do it (adding -supervisor and -0 to a supervisor file name), I'm ok with it since it's a pretty minor point :)
There was a problem hiding this comment.
If you have the intention to do it (adding
-supervisorand-0to a supervisor file name), I'm ok with it since it's a pretty minor point :)
Ahhh, OK. Their file names are intended.
Signed-off-by: Hiroshi Hatake <hatake@clear-code.com>
cosmo0920
force-pushed
the
logrotate-windows
branch
from
5efe76a
to
fa0348d
Compare
|
Thanks for the patch! |
Support logrotate on Windows correctly. Fix #2446 Signed-off-by: Masahiro Nakagawa <repeatedly@gmail.com>
When log rotation is enabled on Windows, the log path is separated for each process. But, the new path value is not applied to the instance variable, so it breaks the following behavior, since fluent#2663. * `File.chown(chuid, chgid, @path)` * `@logdev.reopen(@path, "a")`. I can't assume that `File.chown` is used on Windows, but `reopen` must be fixed. Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
Signed-off-by: Hiroshi Hatake hatake@clear-code.com
Which issue(s) this PR fixes:
Fixes #2446
What this PR does / why we need it:
In Windows multi-process environment, we cannot rename a file which is opened by another process.
Then, we should do this:
Docs Changes:
I'll add a note in https://docs.fluentd.org/deployment/logging#log-rotation-setting.
Release Note:
Same as title.