-----Original Message----- From: maria-developers- bounces+wlad=montyprogram.com@lists.launchpad.net [mailto:maria- developers-bounces+wlad=montyprogram.com@lists.launchpad.net] On Behalf Of Sergei Golubchik Sent: Dienstag, 13. März 2012 19:09 To: maria-developers@lists.launchpad.net Subject: Re: [Maria-developers] MDEV-15 Log all SQL errors. in file:///home/hf/wmar/mdev15/
Hi, Holyfoot!
Just two comments, see below. Please fix and push ASAP!
On Mar 13, holyfoot@askmonty.org wrote:
------------------------------------------------------------ revno: 3320 revision-id: holyfoot@askmonty.org-20120313160142-ljcsacc11zb34uu6 parent: sergii@pisem.net-20120309082045-iyihz9ysqo82qb71 committer: Alexey Botchkov <holyfoot@askmonty.org> branch nick: mdev15 timestamp: Tue 2012-03-13 20:01:42 +0400 message: MDEV-15 Log all SQL errors. The logger service added to provide the rotating log
functions.
The SQL_ERROR_LOG plugin created to log the SQL errors usint
Hi, this broke Windows build. Would it be possible to fix it? When looking at the patch, I found some things that IMO could be done better. 1. mysys/my_logger.c extern char *mysql_data_home; extern PSI_mutex_key key_LOCK_logger_service; I think referencing variables that do not belong to mysys in mysys is a layering violation. Can mysys be used without sql or not here. Shall everything that links to mysys link sql as well? I think - If there is a need to store data home and PSI key from sql layer, then this something can be passed via initialization function, rather than used directly. Alternatively: Is the functionality provided by my_logger required in mysys? If not, why it is be there, and can it be moved to where it is used, e.g into sql? 2. LOGGER_HANDLE *logger_open(const char *path, unsigned long size_limit, unsigned int rotations) Is (general case) 32 bit size limit limitation by design ? IF so, why is not "int" used (or int32 if one wants a fancy typename)? Files can be larger than 4GB. Even on 32 bits, and on 64 bit systems with 32 bit longs. Can this be fixed? Thanks, Wlad that
logger service.
=== added file 'include/mysql/service_logger.h' --- a/include/mysql/service_logger.h 1970-01-01 00:00:00 +0000 +++ b/include/mysql/service_logger.h 2012-03-13 16:01:42 +0000 + +typedef struct logger_service_file_st LOGGER_SERVICE_FILE;
small comment: please, rename to LOGGER_HANDLE or LOGGER_DESCRIPTOR, or, say, MYSQL_LOGGER. The point is to avoid the word "FILE", because, in principle, it may log to syslog or to a table or whatever. (and better to avoid SERVICE too, because 1) this structure is not a service, it's a handle, 2) services are supposed to be transparent and invisible, they pretend to be regular function calls)
+maria_declare_plugin(sql_errlog) +{ + MYSQL_AUDIT_PLUGIN, + &descriptor, + "SQL_ERROR_LOG", + "Alexey Botchkov", + "Log SQL level errors to a file with rotation", + PLUGIN_LICENSE_GPL, + sql_error_log_init, + sql_error_log_deinit, + 0x0100, + NULL, + vars, + NULL, + 0
Fix this, please. Two last values are - version (as a string), and maturity. For example,
"1.0", MariaDB_PLUGIN_MATURITY_ALPHA
+} +maria_declare_plugin_end;
Regards, Sergei
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp