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

Please, do not lock the log file #36

Closed
evandrocoan opened this issue Jan 26, 2018 · 21 comments
Closed

Please, do not lock the log file #36

evandrocoan opened this issue Jan 26, 2018 · 21 comments
Assignees
Milestone

Comments

@evandrocoan
Copy link

I use the Python logging module RotatingFileHandler https://docs.python.org/2/library/logging.handlers.html#rotatingfilehandler

Which renames the log file file.log to file.log.1 when it reaches a maximum size like 50MB, however if I am viewing the log file with this tool, the operation fails because the file is locked by

image

@variar
Copy link
Owner

variar commented Jan 26, 2018

The issue is explained upstream: nickbnf#109. File is read from disk for each search and for redrawing. There are performance issues on windows if file is closed and opened each time. File is opened using QFile::open(QIODevice::ReadOnly) and I don't know if one can manage locking that way

@shvez
Copy link

shvez commented Feb 6, 2018

In general Windows allows to open file with delete access. one needs to specify FILE_SHARE_DELETE for CreateFile api call

@stolsvik
Copy link

(Unrelated) @evandrocoan Which tool are your screenshot from?

@evandrocoan
Copy link
Author

@variar variar closed this as completed in f861fc7 Jan 29, 2019
@shvez
Copy link

shvez commented Jan 29, 2019

@variar did you fix this or just closed?

@variar
Copy link
Owner

variar commented Jan 29, 2019

@shvez I hope that the problem is fixed. I've changed file opening on Windows to use native CreateFile function with shareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE. This should allow to rename files that are opened in klogg. If you try to delete file it will get removed when it is closed in klogg.

This commit should be in build >= 19.1.0.322 (https://github.com/variar/klogg/releases/tag/continuous-win). It would be great if someone could test this because I don't use follow file feature and changing log files in my daily job.

@shvez
Copy link

shvez commented Jan 29, 2019

UPD:
@variar I did try and should admit that it does not work as expected :(. Yes, it works as you described but that is something different from what baretail does. Renaming also does not work - access denied :( So we definitely need to use some other trick.

Thanks a lot for this try to make us happy.

@variar
Copy link
Owner

variar commented Jan 29, 2019

Access denied is generated when one is trying to overwrite file opened in klogg. However, renaming opened file works fine. So one could first rename opened file and after that create a new one with old name.

In nickbnf#109 there was a suggestion to add option to actually keep file closed and open it every time some data is required. I'll investigate this solution.

@variar variar reopened this Jan 29, 2019
@variar
Copy link
Owner

variar commented Jan 29, 2019

I added option to keep file closed (see 975c3d7). Can't add ui for it right now, but it should appear in configuration file after closing new version of klogg. After setting perf.keepFileClosed=true in configuration file and reloading klogg it should keep file open only during indexing or reading data. Configuration file is located in %AppData%\klogg\klogg.ini (or klogg.conf near executable for portable version)

The fix is in 19.1.0.233 build.

There could be issues with file changing on disk during index being rebuilt or search in progress. Need more testing in real-life scenarios.

@variar variar self-assigned this Jan 29, 2019
@variar variar added this to the 2019.02 milestone Jan 29, 2019
@variar
Copy link
Owner

variar commented Jan 30, 2019

Since version 19.1.0.235 there is an option in advanced settings to keep file closed as much as possible

@shvez
Copy link

shvez commented Jan 30, 2019

@variar That what I wanted!!. Thank you very much

The only bug I found is a failure after renaming. If I delete opened file, its page cleaned up. But if I rename the file, then its content still on screen, and I get trash if try to search using window below

@variar
Copy link
Owner

variar commented Jan 30, 2019

Great news @shvez, thanks for testing this.

Looks like right now if new file is larger than old one, then only partial index operation is performed for "appended lines", and everything is trashed. Should not happen when overwriting old file with smaller one (that should be the main scenario for rolling log files?)

I'll try to figure out some way to check if new file is really not the same as old one and do full reload (inode checking?)

@shvez
Copy link

shvez commented Jan 30, 2019

In my case, I just renamed a file to something else. Behavior of the tool should be same like file would be deleted

variar added a commit that referenced this issue Jan 30, 2019
@variar
Copy link
Owner

variar commented Jan 30, 2019

Issue with simple renaming of opened file should be fixed in 19.1.0.237.

Overwriting file with larger one completely unrelated file needs further investigation.

@shvez
Copy link

shvez commented Jan 30, 2019

thank you will check it out soon

@wiz0u
Copy link

wiz0u commented Feb 1, 2019

Tested klogg-19.01.0.337-x64-portable.zip : 'Enable Polling' doesn't seem to work. :(
I'm monitoring a log on a network share updated almost every second. Klogg doesn't detect any update to the log after it has been opened.

@variar
Copy link
Owner

variar commented Feb 2, 2019

File monitoring on network shares has not been tested. Seems similar to nickbnf#214. Looks like file monitoring api works with network drives, but only if the remote server supports the functionality. What system are you using as network share server?

@variar
Copy link
Owner

variar commented Feb 2, 2019

@wiz0u I've moved problems with file monitoring and polling to more appropriate issue.

This issue should stay for problems with klogg locking files and preventing other applications access them.

@wiz0u
Copy link

wiz0u commented Feb 4, 2019

Ok for moving to a separate issue, but in my opinion the locking issue made the most sense on network shares where multiple users can view the file through glogg/klogg and possibly prevent daily rolling/archiving/deleting. It is worse than a purely local issue because you cannot tell all users to close their log viewer before going home.

@variar
Copy link
Owner

variar commented Feb 4, 2019

I've a feeling that polling could be locking file for brief period (to get current size and modification time). This might still be breaking rolling logs in rare cases. Randomizing polling time so file is not accessed on "round" time (e.g. at the start of a minute) could help to further reduce locking possibility.

@variar
Copy link
Owner

variar commented Feb 4, 2019

On the other hand polling timer start time is already random. Will close for now.

@variar variar closed this as completed Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants