dwbutler/logstash-logger

Is "sync: true" required (or should be explained) for Redis?

talarczykco opened this issue · 9 comments

I love the gem, but it took me a while to figure out that I needed "sync: true" in order to send logger info to Redis. "Sync" is only mentioned once in the README by way an example for another device, so it took me a while to find it, and that was only by adding bunches of "puts foo.inspect" lines in redis.rb. I might be missing some overall pre-requisite knowledge but should this be made clearer in the documentation? Thanks!

The sync option is available for every device, and means that every log message will be immediately flushed to the output. The default behavior is to buffer messages. The option is not required for any device and should always default to false. If you're not seeing log messages written to Redis, you may not be waiting long enough (or writing enough log messages) for the buffer to flush.

If you're not writing tons of log messages and want to see them immediately, you probably want to set sync to true. Otherwise it's best to leave it at the default value of false.

You're right that the sync option should be better documented in the Readme. It's actually inherited from Ruby's built-in IO#sync option. So if you've worked with Ruby IO before, it's the same concept. I tried to have the Device classes implement as much of the IO interface as possible for the purposes of logging.

I created #45 primarily for the quick testers. I see your point about leaving sync to false for long-running processes, and maybe my suggestion would impact performance for something like Rails, so I'll understand if it's not included as the default in the examples. I also proposed a "logger.close" comment at the end of the example script so people with quick jobs had an easy way to find out how to flush.

FWIW, log rotation in a rails/unicorn environment also requires sync = true. Otherwise, the log file will not be re-opened.

@MichaelDoyle That's interesting. Can you reference any documentation stating this? I couldn't find any.

Also note that if you're using LogStashLogger with Rails, sync will be set to the value of config.autoflush_log. See Railtie source

I find that a bit surprising. I would expect that a flush could be forced before reopening the log file.

If you're using Rails, sync will apparently default to true via the autoflush_log setting. This might be undesirable for other output sources.

I think this deserves further research and documentation. Thanks for bringing this up!

I found some more information from https://unicorn.bogomips.org/TUNING.html:

On POSIX-compliant filesystems, it is safe for multiple threads or processes to append to one log file as long as all the processes are have them unbuffered (File#sync = true) or they are record(line)-buffered in userspace before any writes.

I was not aware of that.

Per #75, it turns out that Unix sockets also default to sync=true. Since Unix sockets are essentially files, this makes sense given the information above.

This is now documented in the Readme. Feel free to add any more comments if any new information arises concerning sync mode.