Merge in_tail_ex features into in_tail. refs #269 #277
Conversation
| end | ||
|
|
||
| config_param :path, :string | ||
| config_param :tag, :string | ||
| config_param :rotate_wait, :time, :default => 5 | ||
| config_param :pos_file, :string, :default => nil | ||
| config_param :read_from_head, :bool, :default => false | ||
| config_param :refresh_interval, :time, :default => nil |
There was a problem hiding this comment.
I think @refresh_interval can have default value (like 30 seconds).
Because anyways watches will be refreshed when fluentd restarts. There're no ways to not refresh watches.
There was a problem hiding this comment.
Because anyways watches will be refreshed when fluentd restarts. There're no ways to not refresh watches.
Could you tell me this situation deeply?
If the user sets /path/to/file, refresh feature is not needed.
So I set nil by default. in_tail refreshes watchers at only start method.
There was a problem hiding this comment.
- If path includes time patterns (like %H)
- All users must set refresh_interval, right?
- Then it's good idea to give default value
- If path doesn't include time patterns
expand_pathsalways return consistent result.- it's ok to have refresh_interval because it does nothing
There was a problem hiding this comment.
in_tail_ex's default is 3600.
I don't know which value is best.
I will check performance with shorter value.
|
@repeatedly Sorry for being late to review this big change. |
| @@ -288,38 +350,55 @@ def on_rotate(io) | |||
| end | |||
| io.seek(pos) | |||
|
|
|||
| @io_handler = IOHandler.new(io, @pe, log, &method(:wrap_receive_lines)) | |||
| @io_handler = IOHandler.new(io, @pe, @log, &method(:wrap_receive_lines)) | |||
There was a problem hiding this comment.
Can here avoid recreating IOHandler? Like: @io_handler.pe = mpe.
There was a problem hiding this comment.
I think here should not recreate IOHandler because @buffer will be lost if we recreate IOHandler.
There was a problem hiding this comment.
Good catch! Will fix soon.
|
If we repeat refresh_interval, pos_file size increases for ever. Usually it doesn't matter because pos_file is small. However, it's better to have a cleanup code. For example, we can set 0xffffffff to pos on |
|
The other code looks good to me 👍 |
|
@frsyuki I added cleaup code for PositionFile. Could you check it again? |
|
I reviewed. The others look good! |
|
I will merge this pull request in half an hour later. |
Merge in_tail_ex features into in_tail. refs #269
|
@repeatedly @frsyuki I am curious, what is the reason for this limitation: "as long as we can notice all users that path pattern must not include rotated files to not cause duplicated records." Is there any way around this? I was hoping to watch stdout files which are being rotated. |
|
@wsorenson Because in_tail can't track file rotation correctly with
Simple way is separating log path. For example: <source>
type tail
path /path/to/rotation/file,/path/to/rotation/never-macth-unique-prefix-*,/path/to/dynamic/*,
</source> |
|
@repeatedly what about the case where the files are simply rotated with copytruncate? I assume it handles this by noticing the underlying file size has gone down? |
|
@wsorenson Yes. in_tail checks inode change or file size reduction. |
Last step of #269 , merge in_tail_ex into in_tail.
The different from original in_tail_ex is below:
refresh_intervalis60by default. The user should setrefresh_interval XXsfor watching new filestrueread_from_headisfalseby default. So you should settruewhen emulates originalin_tail_ex.This pull request changes in_tail internal, so I need some testers.
UPDATED (Mar/28/2014)
refresh_interval changed from
nilto60seconds.