-
Notifications
You must be signed in to change notification settings - Fork 125
Process-sharable log files using O_APPEND/FILE_APPEND_DATA #7
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
Comments
@nblumhardt please explain what you want to achieve. As far as I know, FILE_APPEND_DATA allows write to file without managing current file position and always appends to the end of the file. And possibly there is a tricky situation if data we want to write is bigger than buffer in FileStream. But when do we want to write to log file from different processes? |
Hi Dmitry, sorry about the vague description, just realised it is missing a bit of context. Sharing log files between processes is required in order to use file logging effectively with multi-process IIS applications (even if only one process is running, IIS will use overlapped recycling to keep the old process running while a new one starts up). Using append writes, we should be able to get this sink working better in this scenario, without the caveats of the clunky solutions usually employed. We can get around the limitations of I wasn't initially thinking about the lock when I started investigating this, but since any solution that gets rid of the lock will have to deal with write-atomicity (the main reason the lock's used right now), and because such solutions will need to do some buffer management to maintain efficiency, doing the two things together seems like it might work out nicely... |
@nblumhardt let me intro with my personal opinion - I think writing from multiple processes to a single file is wrong. At least by the following reasons:
Now back to your specific scenario - we use IIS 8.0 and when our site is re-deployed, no matter of "Disable overlapped recycle" setting, it doesn't start another w3wp.exe process, it adds another AppDomain to existing process. So we have no multiple processes. May be I did something wrong, may be other settings affecting this behavior, but it is what I see right now. And anyway, doing atomic append or not, we should get IO out of exclusive lock. I see other people complaining ( serilog/#809 ) about same topic. |
Hey, thanks for the thoughts. Shared file writes have always concerned me as well (and all available historical solutions have been hacky), but it's still a source of many support requests. I don't know about the perf impact of append-writes, but given it's a single system call (and supported in the filesystem on Linux, so possibly on Windows, too) I would be surprised if it's significant vs. the other overheads involved. I guess the conclusion here is yet again "needs experimentation" - I'll give it a shot sometime and see what we end up with. Regarding serilog/serilog#809 - having a richer set of async/background logging options does seem like a good idea, completely on board with that 👍 Just for clarity, the 809 issue is about dealing with network writes, which the existing In other scenarios, synchronous/durable writes to the log file are desired - still enough that it's a reasonable default we can also improve on. As always, just more work to do :-) |
…to non-shared file perf
I just came to this issue by hopping around:
As Serilog is officially mentioned on aspnetcore page and atomic appends seem to be supported on all platforms (I bet its defined in POSIX and even working on SMB shares) maybe you would have more luck in convincing them that such a feature is missing? |
These flags (Unix/Windows) provide atomic append to shared writeable files. Using these flags via .NET's
FileSystemRights.AppendData
,File()
(and thusRollingFile()
) should be able to support the sharing of log files by multiple processes without too many caveats.A brief description of the API can be seen here.
Since this will most likely require us to pre-render each event into a single buffer to write atomically, we should get rid of the (allocation-lean but contention-heavy) IO-under-lock at the same time.
@DmitryNaumov thanks for bringing this up the other day - what do you think of this proposal?
The text was updated successfully, but these errors were encountered: