There is no way it seems to disable the watchdog, which can benefit the performance of busy sites by disabling it.

It currently require commenting out bootstrap.inc, but it would be useful to have it optional, to turn on when there is a need, and turn it off when there is no need.

Comments

kbahey’s picture

Version: 4.6.3 » x.y.z
Status: Active » Needs review
StatusFileSize
new1.07 KB

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

$conf = array (
  'disable_watchdog' => 0,
);

Hope this helps...

Richard Archer’s picture

StatusFileSize
new2.66 KB

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

kbahey’s picture

This one is even better since it does not introduce a new variable and reuses an existing relevant option.

Good one Richard.

+1

chx’s picture

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

kbahey’s picture

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

moshe weitzman’s picture

better yet, tie this into the throttle sysetm so that errors are not written when site under heavy load.

dries’s picture

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

tamarian’s picture

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

killes@www.drop.org’s picture

Well, then you shoudl fix the images...

I am very much inclined to "won't fix" this.

tamarian’s picture

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

kbahey’s picture

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

jose reyero’s picture

+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

chx’s picture

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

naudefj’s picture

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

moshe weitzman’s picture

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


// Mysql
function db_query_delayed($sql, ...) {
  $sql = str_tr($sql, array('INSERT' => 'INSERT DELAYED', 'REPLACE' => 'REPLACE DELAYED');
  return db_query($sql, ...);
}

//PGSQL
function db_query_delayed($sql, ...) {
  return db_query($sql, ...);
}

IMO, this greatly reduces the need for reducing watchdog inserts.

killes@www.drop.org’s picture

Title: Wachdog should be optional due to performance » Watchdog should be optional due to performance

I agree with Moshe.

jose reyero’s picture

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

fgm’s picture

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

killes@www.drop.org’s picture

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

chx’s picture

best would be to combine moshe's solution with mine...

fgm’s picture

killes: "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.

Richard Archer’s picture

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

jose reyero’s picture

StatusFileSize
new4.35 KB

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

dries’s picture

chx: 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?

chx’s picture

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

chx’s picture

Why are they that slow? Because whatever way the DB does inserting it needs locking of some kind and that will be slow.

kbahey’s picture

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

Cvbge’s picture

Why are they that slow? Because whatever way the DB does inserting it needs locking of some kind and that will be slow.

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

Richard Archer’s picture

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

Cvbge’s picture

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

boris mann’s picture

Many 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

dries’s picture

I, 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).

+  if(function_exists('watchdog_log')){
+    watchdog_log($type, $message, $severity , $link);
+  }

could be written as:

module_invoke('watchdog', 'log', ...);

Did you test this with the statistics.module enabled? What happens with the menu items under admin/logs?

jose reyero’s picture

Status: Needs review » Needs work

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

Jaza’s picture

Version: x.y.z » 6.x-dev
Component: base system » watchdog.module

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

FiReaNGeL’s picture

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

wim leers’s picture

Status: Needs work » Closed (fixed)

In Drupal 6, watchdog ("Watchdog") is renamed to dblog ("Database logging") and can be disabled. It's enabled by default.

That makes this issue obsolete.