Closed (fixed)
Project:
Drupal core
Version:
6.x-dev
Component:
watchdog.module
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
29 Sep 2005 at 15:07 UTC
Updated:
3 Jul 2007 at 19:14 UTC
Jump to comment: Most recent file
Comments
Comment #1
kbahey commentedHere is a simple patch that will provide a configurable option to disable the watchdog.
Since this is an advanced option, there is no need for a user interface for it.
Instead, you disable watchdog by changing settings.php to have:
Hope this helps...
Comment #2
Richard Archer commentedA user interface can easily and unobtrusively be implemented by adding an option to the Error Reporting selector in admin > settings, Error Handling group.
Patch attached.
Comment #3
kbahey commentedThis one is even better since it does not introduce a new variable and reuses an existing relevant option.
Good one Richard.
+1
Comment #4
chx commentedReview tips: a) is there a peformance hit -- you added something to an often-called function. ? b) does it even apply? I also saw that the UI solution is elegant but I do not have the time to benchmark it and it's not likely to hit core if it's not benchmarked.
Make those reviews meaningful.
Comment #5
kbahey commentedI think an "if" is definitely cheaper than an "INSERT", since the latter is I/O bound, not CPU bound ...
The proof is in the pudding though, needs to be benchmarked ...
A side benefit is disk space usage too ...
Comment #6
moshe weitzman commentedbetter yet, tie this into the throttle sysetm so that errors are not written when site under heavy load.
Comment #7
dries commentedI'm not sure I understand what problem we're trying to fix. Inserting a row in the watchdog is fast, and won't affect performance. If performance is a problem, one should focus on performance bottlenecks and not on an occasional query that is inherently fast.
Comment #8
tamarian commentedIt's more than an occasional single query. All it takes is some broken images, and it will generate 10+ (depending on the number of errros) watchdog insert queries per page view.
Comment #9
killes@www.drop.org commentedWell, then you shoudl fix the images...
I am very much inclined to "won't fix" this.
Comment #10
tamarian commentedYes the images should be fixed, but the DOS should be prevented, in case an author screws up some URLs. It doesn't make sense to duplicate the apache log work, and add it to the MySQL load.
In any case, I have disabled the watchdog before opening the issue, but thought to suggest it here as a performance enhancements, and preventing a possibly ugly situation.
Peace.
Comment #11
kbahey commentedThis is a valid request: provide the option to turn off something, rather than force it down the throat of everyone. I realize that most find this useful, but it can be problematic.
Broken images (and CSS) may not be the site owner's fault, it may be stupid crawlers or something else.
For example, check this pending issue that never wants to make it in core http://drupal.org/node/13148 and you will see how the problem can be aggravated.
Comment #12
jose reyero commented+1 to allow disabling watchdog.
But let's do it right. Why should watchdog be a required module in the first place?
I'm for allowing disabling/enabling watchdog module. If some more people thinks the same I'll produce the patch
Comment #13
chx commentedOn NowPublic.com I had this problem too. After memcaching the hell out of everything (will write and post code), the inserts stood out like a sore thumb. I added an array which contains the types of errors we are not interested in and added
if (isset($not[$type])) return;. I have no UI for this but if you have such a high traffic site that these INSERTs matter, then you can surely maintain an array in $conf, right?Comment #14
naudefj commentedAnother solution might be to use INSERT DELAYED on the watchdog table:
INSERT DELAYED INTO {watchdog}...Problem is that INSERT DELAYED is specific to MySQL and is not supported by any other database.
Comment #15
moshe weitzman commentedI think that INSERT DELAYED would be be a huge win for mysql users for watchdog and stat logging and so on. I suggest we create a function in database API and make the mysql implementation smart and pgsql dumb. example:
IMO, this greatly reduces the need for reducing watchdog inserts.
Comment #16
killes@www.drop.org commentedI agree with Moshe.
Comment #17
jose reyero commentedI agree in that it would be nice to have more functions in the database layer. Like insert_delayed, or a simple insert that handles auto increments by itself and maybe much more, but...
About watchdog, I still think it should be allowed to disable it. I was working in the patch, which is not difficult, but the main problem I run into is what to show up as the default administration page. Any idea?
Comment #18
fgmIt might be even more efficient to add something along the lines of db_insert_deferred($query, ...) which would just record the inserts to be performed in a static arrray, then output them as part of the page termination code, by reordering the possibly interlaced inserts to the same table to perform them in just one query, so that each table due to receive inserts receives them in just one query.
This would work with any DB, as it does not require SQL extensions, and would limit latency due to there being only one C/S query being performed for each table.
Comment #19
killes@www.drop.org commentedfgm: a lot of people use watchdog to analyse what is going on while a page loads. If php aborts then your solution would not give them any hint. We should at least have a way to allow for immediate execution of inserts.
Comment #20
chx commentedbest would be to combine moshe's solution with mine...
Comment #21
fgmkilles: "a lot of people use watchdog to analyse what is going on while a page loads. If php aborts then your solution would not give them any hint. We should at least have a way to allow for immediate execution of inserts."
This could be remedied by having a setting so that deferred inserts are not actually deferred (for debugging situations), but could be returned to deferred again by just toggling that switch, to restore system performance when not debugging.
In addition, since the idea of doing these "lazy" inserts (to use standard OS terminology) requires a flushing call when terminating the script, this can be done in a function that *could* be optionally added during development work by module developers. Much like a "sync()" syscall on UNIX: most of the time you don't need it, and all code takes advantage of the lazy write/write behind behaviour when it exists, but code can call sync() if it needs some level of certainty of a write to disk.
Not forgetting that this is more portable than any deferred syntax based only on one specific engine.
Comment #22
Richard Archer commentedThe point of this issue is that users who don't need the watchdog should be able to turn it off.
If an admin wants to watch as a page loads they can re-enable watchdog, do their analysis then turn it back off.
Comment #23
jose reyero commentedAs Richard says, this is about enabling/disabling watchdog.
And I inisist, why cant we disable watchdog like any other module?
This is quite straight forward, replacing the watchdog function by a wrapper.
However, the main problem I've run into is which page to show as the deffault administration page, and I did as follows, in order, just thinking about which ones can be more useful:
1. watchog logs, if watchdog enabled
2. content administration page, if node enabled
3. system settings.
Using drupal_goto for this also addresses one small usability issue, which was showing the watchdog logs page but not showing the 'logs' link in the menu as 'active'.
Comment #24
dries commentedchx: can you elaborate on the watchdog problem you observed? I'd like to understand why it is such a performance problem. What is causing excessive inserts, and why are they that slow?
Comment #25
chx commentedInserts are always costly, that's without a doubt. More so on postgresql. Hammering the database with floods of 404 messages that some broken bots created is an utter waste. Or aggregator messages...
Given a certain amount of traffic you want only the 'real problem' messages, because otherwise it's "messages flood in" -- "cron deletes watchdog before it goes too large" and bumm! you saw nothing.
Comment #26
chx commentedWhy are they that slow? Because whatever way the DB does inserting it needs locking of some kind and that will be slow.
Comment #27
kbahey commentedDries/chx
This brings us back to this issue http://drupal.org/node/13148.
Lots of 404s = lots of inserts = watchdog becomes a bottleneck.
As far as the issue at hand, I think Richard's patch in #2 above is the simplest and best approach so far.
By default, watchdog is on, and can be turned off if someone wants that, from a user option.
Insert delayed is worth exploring, but may be overkill here.
Comment #28
Cvbge commentedPostgreSQL by default (http://archives.postgresql.org/pgsql-php/2005-03/msg00057.php) and AFAIK MySQL with InnoDB do not need locking on INSERTS.
You should try altering the watchdog database to innodb and checking if the problem still occures.
OTOH INSERT DELAYED pushes this even further, by performing the real insert "when time permits" so might be even faster. But if the problem is really caused by locks on inserts, altering to innodb should fix it.
Comment #29
Richard Archer commentedWhether or not the inserts are slow, if the admin has no use for the data it is a waste of resources collecting and storing it.
On one simple Drupal site I host, 7% of the database disk space is consumed by the watchdog logs. I have better uses for this disk space, the CPU time used to store the data, the RAM consumed by the database index etc.
And the logs contain nothing of interest to me. All the 404 errors are especially nauseating because I already have infrastructure in place to analyse them from the web server logs.
Comment #30
Cvbge commentedWhat about some more advanced watchdog controll? For example "I don't care about 404 pages, but I'd like to see my custom module logs". So you should be able to specify message type which are logged, maybe user/role who triggered it ("log only 'site webaster' users"), maybe something else?
Comment #31
boris mann commentedMany multi site users will want to turn off Drupal features that add no value since they are handled at a different layer of the stack -- e.g. watchdog for errors, statistics for logs.
I don't think watchdog provides any crucial functions, so it should be disable-able. I like Jose's fall back of default pages to display as well. +1
Comment #32
dries commentedI, too, tend to ignore 404s and wouldn't mind to see them pruned. On my personal website, I commented out the call to watchdog() in drupal_not_found(). It's just too intrusive, even on a new site.
Richard's patch in #2 is somewhat confusing; it seems to suggest that the watchdog is solely for error reporting (Do not write errors at all.). That change seems to disable all reporting, not just warnings and errors. Also, it appears if you would end up with an empty admin page, not?
Jose's patch in #23 is nicer in that you make the main entry page a tad more useful. Ideally, I'd be able to have some kind of 'dashboard' that shows me exactly what needs my attention, and why. Jose's approach is also an all or nothing approach which, IMO, is somewhat unfortunate as there are quite a few important watchdog messages that I wouldn't want to miss (unlike the 404s) and that save me some clicking around (eg. are there new comments that need approval, are there new users that need approval, have there been security related issues, etc).
could be written as:
Did you test this with the statistics.module enabled? What happens with the menu items under admin/logs?
Comment #33
jose reyero commentedDries> Jose's approach is also an all or nothing approach which, IMO, is somewhat unfortunate as there are quite a few important watchdog messages that I wouldn't want to miss...
Well, the question is 'why cant we disable watchdog'? You may want no logging at all for very simple sites, so enforcing a module doesn't seem reasonable to me - If only I had a way to set up these mandatory modules trough configuration instead of having it hardcoded...
Dries> Did you test this with the statistics.module enabled? What happens with the menu items under admin/logs?
Yes, they show up directly under 'admin' and though they work, I think they look better grouped together. Any suggestion? A new 'admin/statistics' item?
However, the current one, everything under 'logs' but the 'logs' pointing at watchdog overview doesn't look very intuitive to me. I mean there could be some 'statistics overview' page there.
Waiting for a good idea on how to group the menu items, then I'll repost the patch..
- Ok abt the module_invoke
Comment #34
Jaza commentedMoving to 6.x-dev queue.
As well as allowing watchdog to be disabled, I think we should also have a new config option for watchdog: "log the following types of messages: access denied | content | cron | menu | page not found | php | users". In this list, we should definitely disable 'page not found' logging by default: 404 watchdog messages flood the log of basically every live Drupal site that I have running. Probably most users want logging of most messages, but not 404s (and possibly not cron, 403, etc, as well).
Comment #35
FiReaNGeL commentedAn alternate / acceptable solution could be to at least determine WHAT gets logged. I can find watchdog useful for some critical things, but i cant be bothered to read 'user x logged in / out', 'new content for aggregator source x' when i have 100 feeds, and the extra time it takes to filter out the important messages out of the whole mess...
See http://drupal.org/node/77875 for the suggestion to allow a custom watchdog level. Of course, 'no logging' could also be an option.
Drupal is all about leaving the choice to the user isn't it? :)
Comment #36
wim leersIn Drupal 6, watchdog ("Watchdog") is renamed to dblog ("Database logging") and can be disabled. It's enabled by default.
That makes this issue obsolete.