use mysql merge tables for log tables

I would prefer SELECT an ID as a boundary and do an INSERT ... SELECT then a DELETE. Should be a lot faster... and this method will choke on mysql packet size i am afraid.

Also, why the strtr instead of str_replace?

From Site5.com (I know a lot of people host with them):

MyISAM  	DEFAULT  	Default engine as of MySQL 3.23 with great perform...
HEAP 	YES 	Alias for MEMORY
MEMORY 	YES 	Hash based, stored in memory, useful for temporary...
MERGE 	YES 	Collection of identical MyISAM tables
MRG_MYISAM 	YES 	Alias for MERGE
ISAM 	NO 	Obsolete storage engine, now replaced by MyISAM
MRG_ISAM 	NO 	Obsolete storage engine, now replaced by MERGE
InnoDB 	YES 	Supports transactions, row-level locking, and fore...
INNOBASE 	YES 	Alias for INNODB
BDB 	NO 	Supports transactions and page-level locking
BERKELEYDB 	NO 	Alias for BDB
NDBCLUSTER 	NO 	Clustered, fault-tolerant, memory-based tables
NDB 	NO 	Alias for NDBCLUSTER
EXAMPLE 	NO 	Example storage engine
ARCHIVE 	NO 	Archive storage engine
CSV 	NO 	CSV storage engine
BLACKHOLE 	NO 	Storage engine designed to act as null storage

From Totalchoicehosting.com (another very popular one):

 Engine  	 Support  	 Comment
MyISAM 	DEFAULT 	Default engine as of MySQL 3.23 with great perform...
HEAP 	YES 	Alias for MEMORY
MEMORY 	YES 	Hash based, stored in memory, useful for temporary...
MERGE 	YES 	Collection of identical MyISAM tables
MRG_MYISAM 	YES 	Alias for MERGE
ISAM 	NO 	Obsolete storage engine, now replaced by MyISAM
MRG_ISAM 	NO 	Obsolete storage engine, now replaced by MERGE
InnoDB 	YES 	Supports transactions, row-level locking, and fore...
INNOBASE 	YES 	Alias for INNODB
BDB 	NO 	Supports transactions and page-level locking
BERKELEYDB 	NO 	Alias for BDB
NDBCLUSTER 	NO 	Clustered, fault-tolerant, memory-based tables
NDB 	NO 	Alias for NDBCLUSTER
EXAMPLE 	NO 	Example storage engine
ARCHIVE 	NO 	Archive storage engine
CSV 	NO 	CSV storage engine
BLACKHOLE 	NO 	Storage engine designed to act as null storage

Looks like 2 solid major hosting providers will support this patch.

I've been following the devel thread but looking at the code now--what is the performance impact?

We have 2 tables: one for insert and one for longtime storage. Are writes going to speed up that much when the table is empty verse a large table? If we had lots of indexes on the table sure, that would have to be rebuilt on each insert, but we only have a primary key.

If that is the case, how does this approach compare to just making the log tables InnoDB? Is it faster?

Based on some simple hosting checks, seem either approach could be supported. Maybe we even have a check for InnoDB to make the log tables InnoDB by default if supported, then goto MERGE, then default.

However, that assumes InnoDB is faster. Maybe it's not?

When I try
SHOW ENGINES
or
SHOW TABLE TYPES
in phpmyadmin, I get a SQL syntax error message, both with mysql 4.1.10 and 4.0.
http://dev.mysql.com/doc/refman/4.1/en/show-engines.html

tracking.

Hi killes,

Going through with PostgreSQL stuff and i've decided to report a couple things before i'm done. Firstly, you've named the tables {watchdog_insert} and {watchdog_storage}. Would it not be better to name them {watchdog}_insert and {watchdog}_storage to make it easier when adding prefixes?

Secondly, the variable mysql_can_merge may be more aptly named db_can_merge. All versions of PostgreSQL can do it but I thought it might be better to generalise it.

I'm still going on the pgsql patch.

Cheers,

--
Sammy Spets
Synerger
http://www.synerger.com

There is a major barrier to doing this cleanly. The autoincrement fields are going to cause some issues. In PostgreSQL the sequence is shared between the two tables making it mandatory that the key field is in the insert query. Perhaps you could remove the autoincrement from the _storage table and modify the queries to include the key field. That and the two suggestions above are all that's required before it'll work on PostgreSQL (after you get my patch, of course).

Wheeeeeee!

Also missing are the {} around the table names in the MERGE statement. Here is a patch (rolled it all up in 20 more minutes) for all work so far on both MySQL and PostgreSQL.

ok... @killes has tested my theories on mysql and it all looks good. I've updated my patch with update code. Now i'm a little annoyed that there isn't a better way to go about it in PostgreSQL, but it is how it is.

It turns out two copy operations have to be performed on the data in order to preserve the original association between the table and the sequence. I.e if you choose to do DROP TABLE {watchdog} CASCADE in a future update, the sequence {watchdog}_wid_seq will be dropped automatically. If I was to simply rename the table, create children and copy the data across to the new one, this association would not be preserved. Bit of a PITA, but *shrug*.

I've also added ORDER BY clauses to the MySQL updates. MySQL stuff really needs to be tested as I don't use that platform.

Cheers.

Here is mine for my dedicated server:

mysql> SHOW ENGINES;
+------------+----------+------------------------------------------------------------+
| Engine     | Support  | Comment                                                    |
+------------+----------+------------------------------------------------------------+
| MyISAM     | DEFAULT  | Default engine as of MySQL 3.23 with great performance     |
| HEAP       | YES      | Alias for MEMORY                                           |
| MEMORY     | YES      | Hash based, stored in memory, useful for temporary tables  |
| MERGE      | YES      | Collection of identical MyISAM tables                      |
| MRG_MYISAM | YES      | Alias for MERGE                                            |
| ISAM       | NO       | Obsolete storage engine, now replaced by MyISAM            |
| MRG_ISAM   | NO       | Obsolete storage engine, now replaced by MERGE             |
| InnoDB     | DISABLED | Supports transactions, row-level locking, and foreign keys |
| INNOBASE   | DISABLED | Alias for INNODB                                           |
| BDB        | NO       | Supports transactions and page-level locking               |
| BERKELEYDB | NO       | Alias for BDB                                              |
| NDBCLUSTER | NO       | Clustered, fault-tolerant, memory-based tables             |
| NDB        | NO       | Alias for NDBCLUSTER                                       |
| EXAMPLE    | NO       | Example storage engine                                     |
| ARCHIVE    | NO       | Archive storage engine                                     |
| CSV        | NO       | CSV storage engine                                         |
| BLACKHOLE  | NO       | Storage engine designed to act as null storage             |
+------------+----------+------------------------------------------------------------+

Will test a bit more later.

Did some benchmarking, this method appears to run roughly 10% faster on a Drupal site, assuming log tables have 1M records in them, verse an table with 0 records.

I'll have more details tomorrow... too tired to make sense of those results anymore :-)

Ok so I ran a bunch of tests last night to see if this patch makes sense; it does in theory, but what about practice?

So I set out to test this in the following fashion: I loaded a full Drupal bootstrap, plus all of the logic in index.php. I then wrote some arbitrary code to insert 10 watchdog errors per call... this is simulating a pretty bad site that is generating 10 errors per load :-)

To test, I ran the following benchmark: ab -n 5000 -c 20 localhost:8888/drupalMerge/log_multiple.php

The results:

   ### Empty watchdog table, MyISAM
  
   Trial 1:
   Requests per second:    44.30 [#/sec] (mean)
   Time per request:       22.573 [ms] (mean, across all concurrent requests)

   Trial 2:
   Requests per second:    43.84 [#/sec] (mean)
   Time per request:       22.808 [ms] (mean, across all concurrent requests)


   ### 1M rows in watchdog table, MyISAM

   Trial 1:
   Requests per second:    39.56 [#/sec] (mean)
   Time per request:       25.276 [ms] (mean, across all concurrent requests)

   Trial 2:
   Requests per second:    39.15 [#/sec] (mean)
   Time per request:       25.541 [ms] (mean, across all concurrent requests)


   ### Empty Watchdog table, InnoDB

   Trial 1:
   Requests per second:    42.37 [#/sec] (mean)
   Time per request:       23.599 [ms] (mean, across all concurrent requests)

   Trial 2:
   Requests per second:    42.99 [#/sec] (mean)
   Time per request:       23.260 [ms] (mean, across all concurrent requests)

   Trial 3:
   Requests per second:    44.22 [#/sec] (mean)
   Time per request:       22.613 [ms] (mean, across all concurrent requests)


   ### 1M rows in watchdog table, InnoDB
  
   Trial 1:
   Requests per second:    39.01 [#/sec] (mean)
   Time per request:       25.636 [ms] (mean, across all concurrent requests)

   Trial 2:
   Requests per second:    40.51 [#/sec] (mean)
   Time per request:       24.687 [ms] (mean, across all concurrent requests)

Conclusions:

- InnoDB doesn't seem to be any faster, so keeping the mechanism as MyISAM will be ok
- Inserting records on an empty table is on average 12% faster than compared to one that has 1M records in it

Keeping your watchdog table as empty as possible will net the most gains. Keeping your site from generating lots of log messages will speed up yoursite--so cut down on the errors (can't control lots of simultaneous users and creation of content [that's a good thing])

Seems to be solid evidence for the support of this patch.

Just confirmed with Dries, InnoDB shouldn't be any faster, there are no table locks with the watchdog inserts. Also, he agrees that -n 5000 and -c 20 are good settings to test this patch.

Next up is to actually test this patch and make sure it still applies :-)

No longer applies. Statistics.install needs to be tweaked, along with 1006 -> 1007 a few other minor issues. Unfort I have no time to reroll right now hopefully someone else can in the next day or so, this needs to be committed it's a nice performance tweak for heavy sites :-)

Ted's test secenario is not a realistic one and does not allow us to value the usefulness of this patch.

Generating 10 watchdog entries per requested page obviously skews the results in favor of this patch. We should try the following: do a full bootstrap and generate one watchdog entry per 10 page requests. That is a more realistic scenario (but still too optimistic). If the performance gain is still significant, it is worth the added complexity.

Ok I ran the tests again, but this time, each request only produced *one* watchdog entry. This is very realistic... on average 1 per user makes sense.

Here are the numbers:

ab -n 1000 -c 20 localhost:8888/drupalLog/log_multiple.php

## 1M Rows

Requests per second:    29.87 [#/sec] (mean)
Time per request:       33.479 [ms] (mean, across all concurrent requests)

Requests per second:    26.88 [#/sec] (mean)
Time per request:       37.208 [ms] (mean, across all concurrent requests)

Requests per second:    28.50 [#/sec] (mean)
Time per request:       35.086 [ms] (mean, across all concurrent requests)

Requests per second:    27.93 [#/sec] (mean)
Time per request:       35.810 [ms] (mean, across all concurrent requests)



## Empty

Requests per second:    30.99 [#/sec] (mean)
Time per request:       32.270 [ms] (mean, across all concurrent requests)

Requests per second:    31.16 [#/sec] (mean)
Time per request:       32.092 [ms] (mean, across all concurrent requests)

Requests per second:    32.34 [#/sec] (mean)
Time per request:       30.918 [ms] (mean, across all concurrent requests)

On average the empty table was still about 10% faster.

This is from a cheap bargain priced shared host. Both InnoDB and merge are supported.

mysql> show engines;
+------------+---------+------------------------------------------------------------+
| Engine     | Support | Comment                                                    |
+------------+---------+------------------------------------------------------------+
| MyISAM     | DEFAULT | Default engine as of MySQL 3.23 with great performance     |
| HEAP       | YES     | Alias for MEMORY                                           |
| MEMORY     | YES     | Hash based, stored in memory, useful for temporary tables  |
| MERGE      | YES     | Collection of identical MyISAM tables                      |
| MRG_MYISAM | YES     | Alias for MERGE                                            |
| ISAM       | NO      | Obsolete storage engine, now replaced by MyISAM            |
| MRG_ISAM   | NO      | Obsolete storage engine, now replaced by MERGE             |
| InnoDB     | YES     | Supports transactions, row-level locking, and foreign keys |
| INNOBASE   | YES     | Alias for INNODB                                           |
| BDB        | NO      | Supports transactions and page-level locking               |
| BERKELEYDB | NO      | Alias for BDB                                              |
| NDBCLUSTER | NO      | Clustered, fault-tolerant, memory-based tables             |
| NDB        | NO      | Alias for NDBCLUSTER                                       |
| EXAMPLE    | NO      | Example storage engine                                     |
| ARCHIVE    | NO      | Archive storage engine                                     |
| CSV        | NO      | CSV storage engine                                         |
| BLACKHOLE  | NO      | Storage engine designed to act as null storage             |
+------------+---------+------------------------------------------------------------+
17 rows in set (0.00 sec)

After the watchdog function was made into a hook, and watchdog is renamed to dblog, this patch will need to be rerolled.

Why critical? Because Drupal 6 runs slow like a dog who's bee shot in the leg. Every performance improvement should be marked critical. Drupal is slowly dying a performance death.

This would be blocked by #566548: Add storage engine support to schema API. anyway.

Downgrading all D8 criticals to major per http://drupal.org/node/45111

This won't land any time soon, given that it requires the other issue (#566548: Add storage engine support to schema API.) first.

Just unassigning issues that haven't been developed for a bit in the D8 queue.