On Wed, Oct 21, 2009 at 7:17 AM, Jay Pipes <jay.pi...@sun.com> wrote:
> Joe Daly wrote:
[cut]
>
> 4) I fundamentally disagree with the technique that the patch uses to record
> some statistics. The patch goes into the individual storage engine
> "handlers" (now called Cursors in Drizzle) and does things like this:
>
> int ha_myisam::delete_row(const byte * buf)
> {
>
> statistic_increment(table->in_use->status_var.ha_delete_count,&LOCK_status);
> - return mi_delete(file,buf);
> + int error=mi_delete(file,buf);
> + if (!error) rows_changed++;
> + return error;
> }
5.0 provided no way to add code for any of the handler methods:
delete_row, write_row, update_row, rnd_read, ...
5.1 adds ha_delete_row, ha_write_row, ha_update row so counting those
events doesn't require per-engine changes, but doesn't do the same for
the read methods (rnd_read, ...).
Drizzle Cursors approach nirvana with ha_rnd_read, ... so that all
instrumentation can be done in one place. I have wanted that for a
long time. Nice work.
>
> The problem with the above is that each storage engine must be separately
> touched and affected. A better solution is to modify the code in
> /drizzled/cursor.cc to call a plugin listener interface and have the plugin
> do its work. For instance, here is the ha_delete_row() call in
> /drizzled/cursor.cc:
No need to explain. The Cursor implementation makes stats collection
and any other code I want to add much cleaner.
--
Mark Callaghan
mdcal...@gmail.com
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : drizzle-discuss@lists.launchpad.net
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
