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

sincedb: do not read entire line #51

Merged
merged 2 commits into from
May 28, 2015
Merged

Conversation

buom
Copy link
Contributor

@buom buom commented Mar 1, 2015

The issue occurs when we delete a file and then add a new one -- even if it has a different name -- it may well have the same inode number, and the File handler will think it is the same file. In this case we should reset the position to 0 (the old inode)

@wiibaa
Copy link
Contributor

wiibaa commented Mar 1, 2015

@buom thanks for noticing this bug. I'm just wondering why you reset the sincedb entry to 0 instead of removing the record from the sincedb. Any strong reason ?

@buom
Copy link
Contributor Author

buom commented Mar 2, 2015

@wiibaa I works on filewatch 0.5.1 (ships w/ logstash 1.4.2) and I do not notice that the _sincedb_write function has changed (make temp file then rename to).
So, we should reset the sincedb entry to 0 to fix on ver. 0.5.1 and remove the the record from the sincedb on the current version as your suggestion

@wiibaa
Copy link
Contributor

wiibaa commented Mar 2, 2015

@buom as your PR is targetting master can you make the change to remove the record from the sincedb on the current version please

wiibaa added a commit to wiibaa/ruby-filewatch that referenced this pull request Mar 2, 2015
@wiibaa wiibaa mentioned this pull request Mar 2, 2015
@buom
Copy link
Contributor Author

buom commented Mar 2, 2015

@wiibaa updated

@wiibaa
Copy link
Contributor

wiibaa commented Mar 2, 2015

@buom thanks
@jsvd can you have a look please

@jsvd jsvd self-assigned this Mar 2, 2015
@jsvd
Copy link
Collaborator

jsvd commented May 28, 2015

LGTM

jsvd pushed a commit that referenced this pull request May 28, 2015
jordansissel added a commit that referenced this pull request May 28, 2015
jordansissel added a commit that referenced this pull request May 28, 2015
sincedb: do not read entire line
@jordansissel jordansissel merged commit 46a62ce into jordansissel:master May 28, 2015
@jsvd
Copy link
Collaborator

jsvd commented Aug 6, 2015

@buom can you elaborate a bit more on the scenarios where you saw this issue? We now have noticed that it creates a bug where rotating a file through "mv" (like mv file file.old) makes filewatch read the old file again.

The reason for this PR was that after deleting a file and creating a new one, it could have the same inode thus making it "invisible" to filewatch. Is this correct, @buom ?

@buom
Copy link
Contributor Author

buom commented Aug 7, 2015

@jsvd Yes, it is.

input { 
    file {
        path => "/logstash/OnlineLog.log.*"
        codec => plain { charset => "UTF-8" }
        start_position => "beginning"
        sincedb_path => "/tmp/sincedb.db"
    }
}

cp -f /app/rawlog/2015-08-01/OnlineLog.log.* /logstash/
rm -rf /logstash/*

cp -f /app/rawlog/2015-08-02/OnlineLog.log.* /logstash/
rm -rf /logstash/*

cp -f /app/rawlog/2015-08-03/OnlineLog.log.* /logstash/
rm -rf /logstash/*

...

@jsvd
Copy link
Collaborator

jsvd commented Aug 7, 2015

Not sure how to properly solve this. The issue this PR creates is quite bad as it doubles the data if logrotate does a mv log log.bak because the sincedb position is reset to 0.

@jsvd
Copy link
Collaborator

jsvd commented Aug 7, 2015

@buom maybe we can detect on a new file, if there's an existing inode and the size differs from the new file, we reset that to zero?
This way during a mv log log.bak the new file would have the same size with the same inode and would be left untouched.

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.

None yet

4 participants