Skip to content

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

Closed
nblumhardt opened this issue Jul 9, 2016 · 5 comments
Closed

Process-sharable log files using O_APPEND/FILE_APPEND_DATA #7

nblumhardt opened this issue Jul 9, 2016 · 5 comments

Comments

@nblumhardt
Copy link
Member

These flags (Unix/Windows) provide atomic append to shared writeable files. Using these flags via .NET's FileSystemRights.AppendData, File() (and thus RollingFile()) 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?

@DmitryNaumov
Copy link

@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?

@nblumhardt
Copy link
Member Author

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 FileStream a few different ways (I think) including PInvoke if worst comes to worst.

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...

@DmitryNaumov
Copy link

@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:

  • To every written line you need to add PID discriminator to distinguish which process made this record. It makes reading painful.
  • FILE_APPEND_DATA imo (by gut filling and good reasoning only) slows performance, may be on driver/kernel/OS level, but slows.
  • It makes things complicated, for example if we have rolling file and 2 processes are writing to it, which one will create new file, when existing is "full"?

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.

@nblumhardt
Copy link
Member Author

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 PeriodicBatchingSink handles reasonably well.

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 :-)

@springy76
Copy link

I just came to this issue by hopping around:

  1. Portability analyzer telling me that I use a FileStream constructor in my code which is not supported on netstandard (usage of FileSystemRights enum)
  2. Google
  3. Your blog post
  4. Code here

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?

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

3 participants