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

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

Closed
talarczykco opened this issue May 2, 2015 · 9 comments
Closed

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

talarczykco opened this issue May 2, 2015 · 9 comments
Labels

Comments

@talarczykco
Copy link

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!

@dwbutler
Copy link
Owner

dwbutler commented May 2, 2015

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.

@talarczykco
Copy link
Author

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.

@MichaelDoyle
Copy link

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

@dwbutler
Copy link
Owner

dwbutler commented Oct 9, 2015

@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

@dwbutler
Copy link
Owner

dwbutler commented Oct 9, 2015

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!

@dwbutler
Copy link
Owner

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.

@dwbutler
Copy link
Owner

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.

@dwbutler
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants