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

I'm getting like 300 of these a second in production, please help it's killing my logging #482

Closed
wants to merge 1 commit into from

Conversation

will
Copy link
Contributor

@will will commented Aug 23, 2011

I'm getting like 300 of these a second in production, please help it's killing my logging

@dylanegan
Copy link
Contributor

Y U NO NAME BUCKETS PROPERLY? ;)

Anyways, just talking to Will about this and it might be a good idea to add a display_warn method to Formatador that would handle the if $VERBOSE part. This way you could still have formatted lines, but warnings can be ignored.

@will
Copy link
Contributor Author

will commented Aug 24, 2011

👍

@geemus
Copy link
Member

geemus commented Aug 24, 2011

@dylanegan - Yeah, I was kind of figuring I would work on making this into something where it was using something more like a logger that would have some options. Not sure if that should be on the formatador side or on the fog side though. Will have to consider a bit more (I guess it might make since in formatador though).

@dylanegan
Copy link
Contributor

Moving it to a logger would be even better. Since we can easily tweak setting the log level over $VERBOSE.

@geemus
Copy link
Member

geemus commented Aug 24, 2011

Was contemplating log levels. I know that this is commonly how it is done, but seems inflexible. Was thinking something where you could register levels and then turn them on/off maybe. So you could register a :warning level which would be on by default, but you could turn warnings off and leave :info on if you wanted. Perhaps this is overkill though, would anybody ever want that? Also, is it important to be able to redirect the logs (ie to a file instead of STDOUT)? Trying to figure out how I'd like this setup before I get waste deep in it and lock myself into a particular setup.

@fdr
Copy link

fdr commented Aug 26, 2011

By the way, the warning message mentions performance degradation, but I have (on limited perusal) not really understood why aws prefers one calling convention or another, was performance specifically mentioned by the docs somewhere?

Sigh, as for bucket renames, the story is:

  • Renaming a bucket: FFFFFFFUUUUUUUUUUUU
  • Creating a lower-case bucket by the downcased name: FFFFFFFUUUUUUUUUUUU
  • Deleting a bucket: FFFFFFFUUUUUUUUUUUU

Which means one has to change their buckets everywhere :P

@geemus
Copy link
Member

geemus commented Aug 29, 2011

@fdr I do not know why aws prefers one or the other per se (I don't really have any more insight into their internals than anybody else does). The docs from aws rarely if ever mention performance, I found this out from doing benchmarking and testing over time. If you write a script that does operations on two buckets, one with a dns-able name and one without, the results are pretty damning. Changing buckets is definitely a big pain in the butt, but it is also a pretty big performance difference (if I recall correctly, things were something like 3-4x faster with dns-able names).

@pweldon
Copy link
Contributor

pweldon commented Aug 29, 2011

@fdr, @geemus the performance difference is most likely caused by the additional redirect for every request that happens when you use the generic end-point for non-dns names. Using virtual host style requests with dns names eliminates this redirect as aws has already pre-computed the redirect at the dns level. I think this page explains the basics:
http://docs.amazonwebservices.com/AmazonS3/latest/dev/Redirects.html

@geemus
Copy link
Member

geemus commented Sep 1, 2011

Alright, I wrote up a prototype simplest thing that could work kind of implementation. It will work with anything that responds to #write(value) or nil (to not do anything) and supports multiple channel/levels. Also strips the special characters from non-tty. Could perhaps use some cleanup, but I think it covers the basics and should give enough stuff, wanted to elicit feedback before I go to the trouble of converting existing warning/deprecations to use it (since it seems like it may take a bit more than simple find/replace to properly accomodate). Here is the change for your feedback: geemus@03ce1c3

@will
Copy link
Contributor Author

will commented Sep 1, 2011

So then we would remove the warning channel?

@geemus
Copy link
Member

geemus commented Sep 1, 2011

Yeah, though I realize now that commit left something out. With this update: geemus@e52ae34

You could just do this to turn it off:

Fog::Logger[:warning] = nil

And this to turn it back on:

Fog::Logger[:warning] = STDOUT

Or this to write to a file:

Fog::Logger[:warning] = File.open('warnings.txt', 'a')

@dylanegan
Copy link
Contributor

Looks good to me.

@geemus
Copy link
Member

geemus commented Sep 2, 2011

I'll try to get the Formatador.display_line("[WARN]...[/]") stuff converted over later today (or at least soon) and then get it merged back into master.

@geemus geemus closed this in 37e65f3 Sep 2, 2011
@geemus
Copy link
Member

geemus commented Sep 2, 2011

That stuff is merged to master, it should work as I mentioned up there but do let me know if you run into any edge cases or anything. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants